-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
chore: flag overview page redesign - environments #8683
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
@@ -40,6 +40,7 @@ describe('demo', () => { | |||
res.body.flags = { | |||
...res.body.flags, | |||
demo: true, | |||
flagOverviewRedesign: true, |
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.
What's the implications of 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.
This makes the demo e2e tests, that we only run manually and rarely, use the new flag overview page redesign.
@@ -227,7 +227,6 @@ export const deleteFeatureStrategy_UI = ( | |||
}, | |||
).as('deleteUserStrategy'); | |||
cy.visit(`/projects/${project}/features/${featureToggleName}`); | |||
cy.get('[data-testid=FEATURE_ENVIRONMENT_ACCORDION_development]').click(); |
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.
Is this in combination with the demo.ts change of adding the feature flag that we enable the flag to do the test, so we are only testing the new UI and not the old UI?
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.
With this change we're only testing the new UI, not the old one. See: https://github.com/Unleash/unleash/pull/8683/files#diff-3c93375ad9f5acedec6fbc6c020d3927053759ee0b74bd129a1397e1e22ba81aR47
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.
Nice one, big task! 😅
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.
I see you have a 👍 from David already, but I'll add mine as well. Kudos on doing this 🥇
https://linear.app/unleash/issue/2-2826/enabling-environment-via-feature-flag-environment-section-header
https://linear.app/unleash/issue/2-2825/feature-flag-list-bottom-left-to-be-a-nav-section
Follow-up to: #8663
Implements most of the remaining work for our flag overview page redesign.
Most of the code you see is a straight copy/paste from our older existing components, with the slight improvement here and there.
Includes some improvements to our vertical tabs component to suit our use case.
Also updates the Demo flow accordingly. I did some manual tests and it seems to work decently in both scenarios, whether
flagOverviewRedesign
is enabled or not. The demo needs some love but that's a story for a different PR and a different time.Once again, due to the duplicate file pattern, we should remember to clean this up if we decide to remove the flag.