-
-
Notifications
You must be signed in to change notification settings - Fork 442
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
feat(playground): use Starlight #985
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit, overall LGTM!
✅ Deploy Preview for rad-torte-839a59 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Co-authored-by: Yan Thomas <[email protected]>
Co-authored-by: Yan Thomas <[email protected]>
Co-authored-by: Yan Thomas <[email protected]>
e8bd7dc
to
9383656
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks amazing, great work!
Two super small styling nits:
-
We could change the
accent-color
for the checkboxes to match the link colors, which I believe are prettier.We just need to add this in one of the playground
.scss
files:input[type=checkbox] { accent-color: var(--sl-color-text-accent); }
-
I can still see a scrollbar in the page, perhaps we can
overflow: hidden
the page's body to avoid this since we don't really lose any space from what I tested in mobile and desktop. Perhaps we can fix it without forcingoverflow: hidden
even!
This is such a significant improvement! One thing: we should remove the dropdown for selecting the language because we don't plan to translate the playground, and choosing another language will cause a 404. |
I'll be working on it as a separate PR we can merge before/after this one, will also do some extra work to ensure we avoid 404s in these pages if you're coming from another language. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
@SuperchupuDev Thank you for testing it on windows/linux, can you see if it's ok now? |
With firefox it seems good |
@ematipico i'm using chrome beta |
@SuperchupuDev I couldn't reproduce it using browserling.com (Windows 11 and latest Chrome version). |
I also could not reproduce this one in Chrome 120, it might be a regression in Chrome's beta. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to merge once @SuperchupuDev is happy
it's okay, it's just a very small detail, i don't think it matters that much. it looks really good regardless |
Summary
This PR is part of #881 and focus on migrate the Playground page to Starlight including reuse its color scheme.
Additionally, it includes some cleanup of old files from Rome website.
Test Plan
I've done manually: