Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade docs site to Astro v3 #4461

Merged
merged 16 commits into from
Sep 7, 2023
Merged

Upgrade docs site to Astro v3 #4461

merged 16 commits into from
Sep 7, 2023

Conversation

ElianCodes
Copy link
Member

@ElianCodes ElianCodes commented Aug 31, 2023

What kind of changes does this PR include?

  • Changes to the docs site code

Description

  • upgrades Astro and integrations to v3

Todo:

  • wait on astro-og-canvas to catch up with v3 compatible version
  • import all as types
  • change endpoints to uppercase
  • FileTree fails
  • Make sure everything works

Known Bugs

  • Image source is [object object]
  • Theme not working (localStorage)
  • Sometimes weird tabbing behaviour (localStorage)
  • Some spacing seems off (asides and chevrons) (compressHTML issue)

Preview URL: https://deploy-preview-4461--astro-docs-2.netlify.app/

@netlify
Copy link

netlify bot commented Aug 31, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit a7ddbf5
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64f9d57348f03500084831c6
😎 Deploy Preview https://deploy-preview-4461--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sarah11918 sarah11918 added the site improvement Some thing that improves the website functionality - ask @delucis for help! label Sep 1, 2023
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @ElianCodes! Looks surprisingly mostly OK on a quick browse around.

We might want to set scopedStyleStrategy: 'where' in astro.config.mjs just to be safe. I’ve only spotted one CSS detail off so far caused by specificity shifts with the new attribute default, but there might be some others lurking and shouldn’t do any harm to switch back to the :where() approach.

I haven’t seen anything yet, but we should also keep an eye out for issues caused by compressHTML now being enabled by default. Spotted a few issues in Starlight where spaces between inline elements were getting collapsed due to withastro/compiler#852

@ElianCodes ElianCodes marked this pull request as ready for review September 6, 2023 20:03
@itsmatteomanf
Copy link
Contributor

Very minor thing, but on Safari latest /en/guides/upgrade-to/v3/ doesn't have a fully working scroll-spy (works on current deployed).

Stops at https://deploy-preview-4461--astro-docs-2.netlify.app/en/guides/upgrade-to/v3/#moved-astro-check-now-requires-an-external-package.

@ElianCodes
Copy link
Member Author

Very minor thing, but on Safari latest /en/guides/upgrade-to/v3/ doesn't have a fully working scroll-spy (works on current deployed).

@itsmatteomanf, weird! It seems to work fine on my Safari!

@itsmatteomanf
Copy link
Contributor

itsmatteomanf commented Sep 6, 2023

Another, even more minor thing, the chevrons on the sidebar are closer to the text. Tested on Chrome and Safari latest.

image
image

@itsmatteomanf
Copy link
Contributor

Very minor thing, but on Safari latest /en/guides/upgrade-to/v3/ doesn't have a fully working scroll-spy (works on current deployed).

@itsmatteomanf, weird! It seems to work fine on my Safari!

And now works for me, too. I tried refreshing, changing page, hard refreshing, and it stayed, but now decided to work. It's so interesting sometimes.

@itsmatteomanf
Copy link
Contributor

itsmatteomanf commented Sep 6, 2023

Actually important thing.

The theme selector default loads to unselected.

image

If you select the other option compared to current then works fine, but doesn't save for the next load, which resets to system settings. Both Chrome latest and Safari latest.

Checked, the theme local storage is not set.

@Alynva
Copy link
Contributor

Alynva commented Sep 6, 2023

I found a weird tab synchronization behaviour:

e2888f26-3b4d-4c09-ac0a-35c17a558b3a

@itsmatteomanf
Copy link
Contributor

itsmatteomanf commented Sep 6, 2023

I found a weird tab synchronization behaviour:

That works fine for me, the snap to the top of the code block scroll wise is jarring, but it's the same as v2.

Ok, now I see that, too. It doesn't happen all the time, but it does.

@itsmatteomanf
Copy link
Contributor

the chevrons on the sidebar are closer to the text

Same with the language "tags" for not translated pages.

@sarah11918
Copy link
Member

Not sure whether this was happening before but confirming the site and new port works fine in Gitpod, but with these error messages in the terminal:
image

@ElianCodes ElianCodes self-assigned this Sep 6, 2023
@Alynva
Copy link
Contributor

Alynva commented Sep 6, 2023

I found a weird tab synchronization behaviour:

That works fine for me, the snap to the top of the code block scroll wise is jarring, but it's the same as v2.

Ok, now I see that, too.

I think it happens on components after the page's fold, here its possible to test both (before and after the fold)

@itsmatteomanf
Copy link
Contributor

itsmatteomanf commented Sep 6, 2023

I found a weird tab synchronization behaviour:

That works fine for me, the snap to the top of the code block scroll wise is jarring, but it's the same as v2.
Ok, now I see that, too.

I think it happens on components after the page's fold, here its possible to test both (before and after the fold)

Yeah, it looks like some sort of issue with loading of the component.

Copy link
Member

@Fryuni Fryuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried every behavior I could remember and every combination of odd actions I could think of, with some special care on the tutorials. Everything is behaving correctly as far as I can tell, but I found a single asset problem.

The confirmation for the feedback form is broken.

How it was supposed to be:
image

What happened:
image

@VoxelMC
Copy link
Contributor

VoxelMC commented Sep 6, 2023

Very very minor thing I noticed a little while ago - some faulty styling here.

image

@sarah11918
Copy link
Member

@VoxelMC That issue is in the live docs right now! Sharp eye, but it's not related to the v3 upgrade and needs its own fix.

Would you like to make a separate PR to fix the indentation there? https://github.com/withastro/docs/edit/main/src/content/docs/en/guides/middleware.mdx

@Fryuni
Copy link
Member

Fryuni commented Sep 6, 2023

Seems like the same problem is present in all translations, same fix for all of them.

@VoxelMC
Copy link
Contributor

VoxelMC commented Sep 6, 2023

@VoxelMC That issue is in the live docs right now! Sharp eye, but it's not related to the v3 upgrade and needs its own fix.

Would you like to make a separate PR to fix the indentation there? https://github.com/withastro/docs/edit/main/src/content/docs/en/guides/middleware.mdx

@sarah11918 I will do just that!

@itsmatteomanf
Copy link
Contributor

@ElianCodes looks good now to me, the toggle for light/dark works perfectly.

@github-actions github-actions bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Sep 7, 2023
@ElianCodes ElianCodes requested a review from delucis September 7, 2023 13:31
Copy link
Member

@TheOtterlord TheOtterlord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other client:visible component usage seems to work 👍

@ElianCodes ElianCodes linked an issue Sep 7, 2023 that may be closed by this pull request
3 tasks
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for all the work on this @ElianCodes and everyone for checking in and reporting problems!

@ElianCodes ElianCodes merged commit d96e972 into main Sep 7, 2023
@ElianCodes ElianCodes deleted the elian/experiment-3 branch September 7, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 action i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! site improvement Some thing that improves the website functionality - ask @delucis for help!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI to Astro 3
8 participants