-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
feat: add toggle to show/hide LTS releases #8263
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: add toggle to show/hide LTS releases #8263
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull Request Overview
This PR adds a toggle switch to filter non-LTS releases from the EOL (End of Life) table. Users can now hide non-LTS versions to focus on LTS releases only.
Key Changes:
- Introduced a new Switch UI component with Radix UI integration
- Refactored EOL table to support client-side filtering with state management
- Fixed the
isLtsdetermination logic to useltsStartinstead of status string matching
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/src/Common/Switch/index.tsx | New Switch component implementation using Radix UI primitives |
| packages/ui-components/src/Common/Switch/index.stories.tsx | Storybook stories for the Switch component |
| packages/ui-components/src/Common/Switch/index.module.css | Styling for the Switch component with light/dark mode support |
| packages/ui-components/package.json | Added @radix-ui/react-switch dependency |
| packages/i18n/src/locales/en.json | Added translation key for "Hide non-LTS versions" label |
| apps/site/next-data/generators/releaseData.mjs | Fixed isLts logic to check for ltsStart existence |
| apps/site/components/EOL/EOLReleaseTable/index.tsx | Refactored to use new client component |
| apps/site/components/EOL/EOLReleaseTable/TableClient.tsx | New client component with filtering state and Switch integration |
| apps/site/components/EOL/EOLReleaseTable/TableBody.tsx | Removed file - functionality merged into TableClient |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
avivkeller
left a comment
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.
Wow! Thanks for this!
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.
Pull Request Overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Thanks for the quick review! |
flakey5
left a comment
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.
lgtm for web infra bits, thank you!
AugustinMauroy
left a comment
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.
LGMT !
bmuenzenmeyer
left a comment
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8263 +/- ##
==========================================
- Coverage 76.72% 76.26% -0.46%
==========================================
Files 118 118
Lines 9805 9903 +98
Branches 335 336 +1
==========================================
+ Hits 7523 7553 +30
- Misses 2280 2348 +68
Partials 2 2 ☔ View full report in Codecov by Sentry. |
This comment was marked as outdated.
This comment was marked as outdated.
bjohansebas
left a comment
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.
LGTM, it looks cool to be able to do that
|
Lighthouse Results
|
| @@ -0,0 +1,49 @@ | |||
| 'use client'; | |||
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.
As much as I love Radix! I feel we should make the switch purely with CSS and HTML and have the table entries be affected based on that.
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.
Fair, I changed the implementation to use html/css only. However, the only way I could get it to work was through using has, which is newly available (https://caniuse.com/css-has). Does this work?
ovflowd
left a comment
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.
Appreciate your contribution! Thanks for working on this :)
I've left a few comments.
8fe558b to
070fe96
Compare
8c1eb71 to
4cd0fa0
Compare
|
Thanks for the review @ovflowd, I've changed the implementation to use html/css only. The changes to the data sources is also reverted (as they were wrong in the first place). Could you unblock your review @bmuenzenmeyer ? |
| {eolReleases.map(release => ( | ||
| <Fragment key={release.major}> | ||
| <tr> | ||
| <tr data-lts={!!release.codename}> |
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.
smart!
|
I cannot get prettier to overwrite the current contents, which makes me think the failure is a false positive or caching problem |
Howdy! I'm on work travel, I'll be reviewing this later today or tomorrow! |
ovflowd
left a comment
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.
SGTM besides of one nit :)
* feat: add switch in eol table * feat: translation keys * refactor: client renaming * fix: dont create arrow fn onCheckedChange * chore: use tilde for version * refactor: rename prop * fixes test, but does it? * fix: add use client directive * fix: apply motion safe Co-authored-by: Caner Akdas <canerakdas@gmail.com> Signed-off-by: Efe <dogukankrskl@gmail.com> * refactor: use size instead of w&h Co-authored-by: Caner Akdas <canerakdas@gmail.com> Signed-off-by: Efe <dogukankrskl@gmail.com> * fix: use motion safe Co-authored-by: Caner Akdas <canerakdas@gmail.com> Signed-off-by: Efe <dogukankrskl@gmail.com> * refactor: use import type for types Co-authored-by: Caner Akdas <canerakdas@gmail.com> Signed-off-by: Efe <dogukankrskl@gmail.com> * fix: adjust focus visible ring offset * chore: update pnpm lock * revert: releaseData.mjs changes * revert: tablebody * feat: implement the switch in html/css only * revert: @radix-ui/react-switch install * revert: pnpm.lock * revert: tests * feat: new stories * ci: don't mind me triggering the ci --------- Signed-off-by: Efe <dogukankrskl@gmail.com> Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com> Co-authored-by: Caner Akdas <canerakdas@gmail.com>

Description
Adds a toggle to the EOL table that filters non LTS releases
Validation
Content being filtered:

Other:



Related Issues
Closes #8017
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.