-
Notifications
You must be signed in to change notification settings - Fork 461
feat: segment feature state view #6137
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
base: chore/componentise-feature-override-row
Are you sure you want to change the base?
feat: segment feature state view #6137
Conversation
…ponentise-feature-filters
…ponentise-feature-filters
…ponentise-feature-filters
…eature-state-view
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
Co-authored-by: Zaimwa9 <wadii.zaim@flagsmith.com>
…rs' into chore/componentise-feature-filters
…ponentise-feature-filters # Conflicts: # frontend/web/components/pages/UserPage.tsx
…eature-state-view
matthewelwell
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.
I've tested this and unfortunately it doesn't work. See the screen recording below. It looks like when you try to update a feature for a given segment, it's making a call to update it for an identity.
I would expect it to add a segment override for this segment (and include that information in the confirmation modal).
segment-features-page-bug.mov
|
Ah, ok yes this didn't consider toggling the switch directly vs opening the modal. I think for simplicity sake clicking any part of the row should just open up the segment override tab. I've updated this. We could have that automatically add an override to the list though I'm not sure it's a good idea, creating an override at the point where you can see others / set priority is more simple and makes sense. When I use Flagsmith even on fairly simple projects I'd never want to add a segment override without seeing the full context (other segment overrides for this feature). |
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.
Sorry, small change
frontend/web/components/feature-override/FeatureOverrideRow.tsx
Outdated
Show resolved
Hide resolved
…ent-feature-state-view # Conflicts: # frontend/common/stores/feature-list-store.ts # frontend/web/components/modals/AssociatedSegmentOverrides.js # frontend/web/components/segments/Rule/components/RuleConditionValueInput.tsx
Yeah, I guess this makes sense with the current functionality. It is quite annoying to have to find the segment in the list still after clicking the row though. I wonder if there's an option here to add an extra step to creating a segment override which adds a confirmation button next to the drop down in the modal, then we could pre-fill the drop down when you click the row? Maybe that's overkill, but I do feel that the UX here is a little janky. |
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Places the associated features page with the following
Opening a feature from this view simply opens the feature modal with the segment overrides tab selected.
How did you test this code?
Requires #6156