Skip to content

Feat(UI): add ability to collapse/expand strategies #9548

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wuguishifu
Copy link

@wuguishifu wuguishifu commented Mar 16, 2025

About the changes

  • Added a button to collapse/expand the targeting section of the strategies.
  • When collapsed, the rollout percentage and variants are shown.

Expanded (default):

Screenshot 2025-03-15 at 10 51 58 PM

Collapsed:

Screenshot 2025-03-15 at 10 52 06 PM

Closes #9529

Important files

  • The main functionality is located in the directory /frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/CollapseStrategyIcon

Discussion points

  • Right now it seems like for environments the expanded/collapsed state isn't cached at all so refreshing the page resets the state. I followed this behavior for this PR as well, but maybe instead the state should be stored in cookies so it persists across sessions.

Copy link

vercel bot commented Mar 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 16, 2025 6:03am

Copy link

vercel bot commented Mar 16, 2025

@wuguishifu is attempting to deploy a commit to the unleash-team Team on Vercel.

A member of the Team first needs to authorize it.

@thomasheartman
Copy link
Contributor

Hi, @wuguishifu 👋🏼 Thanks for the contribution 😄 As was mentioned in the issue you linked, we've registered this as a feature request and are talking about it internally at the moment.

As you might have noticed, we're working on a pretty big strategy UI update right now, and this falls squarely in the middle of it (and also aligns with our stated goals of the updated interface). Because we're right in the middle of it, we'd like to take this chance to have our UX team think about this fully (what to show / hide, consequences of showing/hiding, tradeoffs, etc) before we implement it. That also means that we're not going to merge this right now.

But to help us understand what the need for this is, it would be interesting to hear your take on why you want this functionality. Could you tell me what problems this solves for you?

Anyway, because we are right in the middle of an update of the strategy rendering code, any files you touch there are likely to cause conflicts, so to keep us all from going bananas with merge conflicts, I'd recommend waiting a little bit before updating this PR anyway 😅

@gastonfournier gastonfournier moved this from New to For later in Issues and PRs Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: For later
Development

Successfully merging this pull request may close these issues.

Allow to collapse strategy
2 participants