Implements playground results for strategies.
Old design:
New design:
Still left: segments.
I also discovered during this that some of the new hooks (and also some
of the new components) accept deprecated types
(`IFeatureStrategyPayload` in this case). If that should indeed be
deprecated, then we also shouldn't use it in the new hooks / components
if we can avoid it. I'll make a task for it.
---------
Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
- new way of showing strategy variants
- fixed wrapping issue in strategy editing, for a lot of variants
defined (`SplitPreviewSlider.tsx` change)
- aligned difference between API and manually added types
Implements the new strategy list design for default strategies. Moves
the old impl into a legacy file. Also: removes the description from the
strategy item. From my digging, we only showed this for default strategy
items and it didn't really provide any useful information. The only
other place you can add a description is for custom strategies (at least
that I could find), but these are deprecated and we never show the
description when you apply the strategy anyway.
Rendered:
Without the flag (nothing changes):
Fixes a visual bug where envs without release plans would get too much
spacing on the top of their first strategy.
It does this flattening the list of strategies if there are no release
plans. In doing so, I have extracted the strategy list rendering into a
separate component (to make things more legible and re-usable) and have
also removed the FeatureStrategyEmpty component and marked it as
deprecated. In the new designs, you can't expand envs without
strategies, so the component is no longer needed.
Before (what looks like a shadow is actually the extra list being
rendered with a bit of padding):
After:
Moves strategy titles and names onto the same line, as per the new
designs.
In doing so, I've also updated the component to use a more semantic
hgroup with the header being the strategy title if it exists or the
strategy name if not.
The downside of being more semantically correct here is that we need to
know what header level we want the strategy to use. In most cases,
that's 3 (e.g. flag name > environment > strategy, release plan >
milestone > strategy), but for plans on flag envs, it's 4 (flag name >
env > milestone name > strategy).
I've also taken the opportunity to fix a little mistake I made earlier.
`ol`s can only have `li` children, and I'd forgotten to wrap a nested
`ol` inside an `li`. The changes in `EnvironmentAccordionBody` all
relate to that change. Because we now have several layers of lists
nested within each other, dealing with styling and padding gets a little
tricky, but CSS has the power do help us out here.
Rendered:
Use new design for release plans in flag environments.
- Move old ReleasePlanMilestone into Legacy file and update imports
- In the new version, use the same strategy list and item as in the
general strategy list and milestone template creation (components to be
extracted in the future)
- Fix an issue with the border being obscured by overflow by hiding
overflow
Here's an initial first pass of replacing the strategy lists in release
plan milestones.
The existing MilestoneCard has been moved to a Legacy file to avoid
conflicts.
This PR places the strategies in a list and changes the background color
of the list items (the strategies themselves still have a white
background, however).
It also re-orders the buttons in the footer and places the
milestone-level drag handle outside the milestone card.
## For later
Changing out the strategy list item itself hasn't been done yet. I want
to see if we can re-use the existing strategy draggable item instead of
making a copy. There's some dependencies on project path params etc that
need to be worked out first, though, so I'd prefer to do get these
initial changes through first.
Removes the `() => {} as any` args from the StrategyDraggableItem
invocation when you have paginated strats. Instead makes all the drag
params optional. It defaults to a no op if not provided.
Also, the reason it had to be typed as `any` before is probably because
it was missing a function. The correct empty param is `() => () => {}` 💁
Adjusts styling of the env dropdown now that we have both release plans
and strategies.
Key points:
- simplifies strategy separator, removes inherent height. Also: extracts
it from the draggable component (it has no business knowing whether to
add that or not)
- Puts release plans and strategies in the same list so that it becomes:
```markdown
- Release plan
- strategy 1
- strategy 2
- (OR) Strategy A
- (OR) Strategy B
```
- Adjusts some padding around to make it line up properly
- Swaps a couple conditional renders for ternaries
Rendered:
## Still todo:
Handle cases where you have >50 strats and we show the warning etc. It's
a little trickier because of how it interacts with release plans, so I
wanna leave that for later.
I'm also unsure about how we handle spacing today. All the little items
have their own different spacing and I'm not sure it won't get out of
sync, but I'm also not sure how else to handle it. We should look at it
later.
Splits the release plan component into a Legacy component and a new one
with the initial changes for the new strategy list view.
Here's what it looks like:
Notice that the background color stops a little early (before the OR
token). I'll handle that in a follow-up because the changes also impact
how the rest of the env accordion body is rendered.
Improves the semantic correctness of the strategy list by wrapping it
in an `ol` tag.
Strategy order matters (due to variant resolution etc), so the order
is important (hence the `ol` instead of a `ul`).
Dragging still works and it's visually the same.
Updates the strategy list based on the new designs and moves the current
versions of the touched components into `Legacy...` files (the vast
majority of changes are that and updating imports). The relevant changes
to the components are listed in their original files.
Flag on:
Flag off:
## Next steps
There's two items to review for improving these current comments (also
noted inline):
- Whether to aria-hide the "or" separator or not (I need to read up a
bit and think whether it makes sense to show that or not)
- Changing the list of strategies into an actual ordered list (`ol`).
That'd reflect the semantics better.
Next would be checking the other places we use strategy lists and then
updating those too. In doing so, I might find that some things need to
be updated, but I'll handle those when I get there.
There's also handling release plans.
Makes the env selector on the flag page act the same way as the env
selector on the new project page or any of the filterable buttons in the
new project/flag dialogs.
Also slightly changes the styles of the existing dropdown lists to bring
them in line with the new env selector (more padding, full-width
highlights).
Selector:
Project/flag creation:
Before:
After:
## Technical notes
I was a little unsure how best to share the padding/spacing styles
between the search field and popover at first (as was requested by UX).
The easiest way (and most compliant with how we do it today) was to
define the spacing in a variable and move the relevant components into
the same file.
However, I actually think that using a CSS variable (e.g.
`--popover-spacing`) would be "better" here, but we don't really use
them much, so I've left that out for now. That said, if you agree, I'd
be more than happy to use that instead 🙋🏼
Reduces the size of the tab buttons on the flag page:
- Sets the min width to 100px instead of 160px on md screens. No change
for smaller screens
- Removes the min-height restriction imposed by theme.ts for the tab
bar, instead relying on the tab buttons to determine the height
(effectively changes the height from 70px to 62px).
Additionally: fixes an issue where the action buttons would overlap with
the tab buttons before wrapping and makes the tab bar scrollable. I can
no longer reproduce the issue where the action buttons force the tab bar
to be too small, but even if they should do that now, the tab bar is
scrollable so the remaining tabs are still accessible.
Because we only override the tabs' min-width on wider screens and mui
sets a default min-width, I changed the `onNarrowHeader` function to
`onWideHeader` and adjusted the other components accordingly. As a
bonus, the tab width and header wrapping now happens at the same time 🥳
After the change:
## Accessibility
This PR also addresses some of the a11y issues with this tab bar, namely
that it adds an `aria-label`, as mentioned in the [MUI
docs](https://v5.mui.com/material-ui/react-tabs/#accessibility).
It does **not**, however, connect the tabs to their corresponding tab
panels. The main reason for this is that we're not using tab panels and
that they're spread over 4 different components. We're probably using
the tabs component for something it isn't really designed to do in this
way. (Arguably they should be links and not buttons, for instance.) I'm
not going to touch this now, because that would probably be a lot of
work and it's not something I expect the business would prioritize.
## Changing theme.ts
While it's tempting to go in and change the `min-height` in `theme.ts`,
that would potentially affect all the other tab bars we have (although
maybe not, because we set a different min height for the tabs
themselves), I want to leave that for now. There is apparently some work
being done/prepared for the tabs, so it's probably better to leave that
for then.
Makes it so that the actions/tabs wrap on narrow width screens.
Constraints:
- When wrapping, the actions should go **before** the tabs, when not
wrapping, they should be placed **after**
- Need to maintain a logical tab order for wrapping, so just using
`flex-flow: row wrap-reverse` doesn't work because the tab order will be
wrong
- When the elements switch, you shouldn't lose your tab place in the
document
This solution uses container queries to determine the container size and
uses that to set the wrapping. Falls back to media queries if container
queries aren't supported (it's supported on >93% of browsers according
to caniuse).
The wrapping points don't use predefined media queries because:
- containers don't care about the size of the screen. It's the intrinsic
size of the container that matters.
- wrapping at 900px seemed too far out if container queries are
unsupported. But it's a fallback, so we can switch to that if we want.
If your keyboard focus is on one of the actions on a wide screen, and
the screen goes narrow, your focus will still be after the tabs (staying
consistent), so tabbing to the next element will take you into the flag
details, while backtab takes you back to the tabs.
Before wrapping:
After wrapping:
## A note on accessibility:
The spec for flexbox (taken from [MDN's
doc](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Ordering_flex_items))
states:
> "Authors must not use order or the *-reverse values of
[flex-flow](https://developer.mozilla.org/en-US/docs/Web/CSS/flex-flow)/flex-direction
as a substitute for correct source ordering, as that can ruin the
accessibility of the document."
So even if wrap-reverse works visually, it's not a good solution for
this.
- Button to show and hide environments
- Refactored hook storing state of hidden environments
- Changed the way flag is triggered for feature overview
- Visual updates for new page look
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
This PR moves the flag page header into a separate file, so that the
overview file is more clearly focused on the overview.
Additionally, it moves the modals that are triggered from the header
into the new file. This should give a nice little performance boost, as
opening and closing these modals should no longer trigger a re-rendering
of the full flag overview page, only the header.
Fixes the height discrepancy between add strategy and more strategies
buttons, both with and without the flag enabled.
The essence of the fix is to make the "more strategies" button's height
dynamic and grow to match the height of the other button.
Before (flag enabled):
After (flag enabled):
Before (flag disabled):
After (flag disabled):
As a bonus: also enables the ui font redesign flag for server-dev.
If you're very sharp-eyed, you might notice a few things:
1. There's more padding on the new button. This was done in concert with
UX when we noticed there was more padding on other buttons. So as a
result, we set the button type to the default instead of "small".
1. The kebab button isn't perfectly square with the flag on. There's a
few issues here, but essentially: to use `aspect-ratio: 1`, you need
either a height or a width set. Because we want everything here to be
auto-generated (use the button's intrinsic height), I couldn't make it
work. In the end, I think this is close enough. If you have other ideas,
you're very welcome to try and fix it.
https://linear.app/unleash/issue/2-2834/plausible
Adds the following Plausible events to the Release management feature:
- Add plan
- Start milestone
- Remove plan
- Create template
- Edit template
- Delete template
This PR fixes a bug wherein the list of tags to remove from a group of
tags wouldn't be correctly updated.
## Repro steps
- Add a console log line to
`frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx`'s
`ManagebulkTagsDialog`. Log the value of the`payload` variable.
- Pick a flag with no tags.
- Add tag A -> before submitting, you should have one added tag and zero
removed flags. After submitting, both should be empty.
- Now remove tag A -> before submitting, you should have one removed tag
and zero added tag. After submitting, both should be empty
- Notice that removed flags hasn't been emptied, but still contains tag
A.
- Now add tab B -> before submitting, you should have tag B in added and
nothing in removed. Notice that tag A is still in removed.
## Discussion points
This gives us both a `clear` and a `reset` event, which is unfortunate
because they sound like they do the same thing. I'd suggest renaming the
`clear` event (because it doesn't really clear the state completely),
but I'm not sure to what. Happy to do that if you have a suggestion.
I have not tested that submission of the form actually resets the state.
I spent about 45 minutes looking at it, but couldn't find a way that was
sensible and worked (considered spying: couldn't make it work;
considered refactoring and extracting components: think that's too much
of a change). I think this is benign enough that it can go without a
test for that thing actually being called.
I did, however, test the different reducer commands.
As of PR #8935, we no longer support both text and title, and confetti
has been removed.
This PR:
- removes `confetti` from the toast interface
- merges `text` and `title` into `text` and updates its uses across the
codebase.
- readjusts the text where necessary.
This PR removes all references to the `featuresExportImport` flag.
The flag was introduced in [PR
#3411](https://github.com/Unleash/unleash/pull/3411) on March 29th 2023,
and the flag was archived on April 3rd. The flag has always defaulted to
true.
We've looked at the project that introduced the flag and have spoken to CS about it: we can find no reason to keep the flag around. So well remove it now.
https://linear.app/unleash/issue/2-3038/release-plans-misc-ux-improvements
Includes various UX improvements focused on release plans:
- **New milestone status:** Introduced a "Paused" status for milestones.
A milestone is marked as "Paused" when it is active but the associated
environment is disabled.
- **Status display:** Paused milestones are labeled as "Paused (disabled
in environment)" for clarity.
- **Styling cleanup:** Removed unused disabled styling in the release
plan component.
- **Accordion stability:** Fixed visual shifting in milestone accordions
when toggling.
- **Strategy count:** Updated the "View Strategies" label to reflect the
total number of strategies in the milestone.
- **Edge case handling:** Improved rendering for milestones without
strategies.
- **Component extraction:** Refactored milestone status into a
standalone component.
- **Component organization:** Grouped milestone-specific components
under a `ReleasePlanMilestone` parent folder.
- **Template card cursor enhancement:** Set the cursor on the template
card to "pointer", so we better reflect the interactivity of the
element.
- **Template card created by enhancement:** Added an avatar for the
"Created by" field in release plan template cards, replacing the
creator's ID.
- **Navigation improvement:** After creating or editing a release plan
template, users are now redirected back to the release management page.
Hooks up the project status lifecycle data to the UI. Adds some minor
refactoring as part of that effort.
## Other files
There's been some small changes to
`frontend/src/component/feature/FeatureView/FeatureOverview/FeatureLifecycle/FeatureLifecycleStageIcon.tsx`
and `frontend/src/hooks/useLoading.ts` as well to accommodate their
usage here and to remove unused stuff. The inline comments mention the
same thing but for posterity (especially after this is merged), the
comments are:
For
`frontend/src/component/feature/FeatureView/FeatureOverview/FeatureLifecycle/FeatureLifecycleStageIcon.tsx`:
> The icon only needs the name to pick.
https://github.com/Unleash/unleash/pull/7049 deliberately changed the
logic so that the completed stage gets the same icon regardless of its
status. As such, to make the icon easier to use other places (such as in
the lifecycle widget), we'll only require the name.
For `frontend/src/hooks/useLoading.ts`:
> There's no reason we should only be able to put refs on divs, as far
as I'm aware. TS was complaining that that a `ul` couldn't hold a div
reference, so I gave it a type parameter that defaults to the old
version.
Fixes browser console warnings and errors related to the event timeline
and strategy form.
- **Event Timeline**: Addressed a warning where the environment filter
rendered with a default environment value (production) before
environments were fully loaded.
- **Strategy Form**: Resolved an error caused by forwarding the enabled
prop as a boolean.
Addressing some oversights that led to browser console errors.
This PR fixes console errors related to the recently introduced
highlight component (#8643) and tag row component in the new flag
metadata panel (#8663).
The two lints being turned off are new for 1.9.x and caused a massive
diff inside frontend if activated. To reduce impact, these were turned off for
the merge. We might want to look at turning them back on once we're
ready to have a semantic / a11y refactor of our frontend.
Fixes all warnings about the "key" prop. The majority of the fixes fall
into one of the following categories:
- Extracting "key" props in tables (you're not allowed to just spread
them in)
- Adding "key" props to autocomplete options and chips
- fixing test data that didn't contain ids
don't use `act` from `react-dom`. Instead, use act from `react`
directly, as advised by the deprecation notice.
This PR fixes all of the deprecated import warnings, updates some
testing libraries we use (and tests), and fixes one or two other
warnings.
Allow you to edit default strategies in the UI if you have the
update_project or project_default_strategy_write permissions. These are
the same permissions that we use in the API.
Previously, we used the update_feature_strategy permission here, but
that one is intended to be used for updating strategies belonging to
actual flags.
One of the trickier bits here is that we use the `StrategyVariants`
component, which previously had baked in the permission required
(update_feature_environment_variants). Because the permissions are
different for the default strategy, I updated the component to make it
configurable, but for the default to be the old permission (so that
other uses aren't affected).
After we implemented new feature flag creation flow, this are not used
anymore.
Creation is now handled by **CreateFeatureDialog**.
Also edit component can be minified, because it does not need so many
fields anymore.
