This PR cleans up the improvedJsonDiff flag. These changes were
automatically generated by AI and should be reviewed carefully.
Fixes#10483
## 🧹 AI Flag Cleanup Summary
This PR removes the `improvedJsonDiff` feature flag, making the enhanced
JSON
diffing component the default and only option. The now-unused legacy
diff
component and all related feature flag logic have been removed to
streamline the
codebase.
### 🚮 Removed
- **Components**
- `OldEventDiff` component was removed, along with its helper types and
constants.
- **Flag Logic**
- All conditional rendering based on the `improvedJsonDiff` flag was
removed.
- The `sort` prop from `EventDiff` was removed as it was only used by
the
legacy component.
- **Configuration**
- `improvedJsonDiff` flag definition was removed from `uiConfig.ts`,
`experimental.ts`, and `server-dev.ts`.
- **Tests**
- Mock configuration for `improvedJsonDiff` in tests was removed.
### 🛠 Kept
- **Components**
- `NewEventDiff` was renamed to `EventDiff` and is now the standard
implementation.
### 📝 Why
The `improvedJsonDiff` feature flag was marked as completed with its
outcome
being "kept". This cleanup finalizes the feature rollout by removing the
flag
and associated legacy code, simplifying the implementation and reducing
code
complexity.
---------
Co-authored-by: unleash-bot <194219037+unleash-bot[bot]@users.noreply.github.com>
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
Adds a timestamp for each state we have time (and that isn't a state
downstream from the current state) in the CR timeline.
<img width="437" height="318" alt="image"
src="https://github.com/user-attachments/assets/a499e73f-c506-46a0-8d1a-7e4eb5ec4f7d"
/>
The timestamp respects the user's preferred locale and uses the `time`
element.
I've used the current name of the API payload's timestamps as it stands
on the enterprise main branch for now. This name is not set in any
schemas yet, so it is likely to change. Because it's not currently
exposed to any users, that will not cause any issues. Name suggestions
are welcome if you have them.
We only show timestamps for states up to and including the current
state. Timestamps for downstream states (such as "approved" if you're in
the "in review" state), will not be shown, even if they exist in the
payload. (There are some decisions to make on whether to include these
in the payload at all or not.)
There's no flags in this PR. They're not necessary If the API payload
doesn't contain the timestamp, we just don't render the timestamp, and
the timeline looks the way it always did:
<img width="447" height="399" alt="image"
src="https://github.com/user-attachments/assets/0062393a-190c-4099-bc16-29f9da82e7ea"
/>
## Bonus work
In the `ChangeRequestTimeline.tsx` file, I've made a few extra changes:
- `createTimelineItem` and `createScheduledTimelineItem` have become
normal React components (`TimelineItem` and `ScheduledTimelineItem`) and
are being called as such (in accordance with [React
recommendations](https://react.dev/reference/rules/react-calls-components-and-hooks#never-call-component-functions-directly)).
- I've updated the subtitles for schedules to also use the time element,
to improve HTML structure.
## Outstanding work
There's a few things that still need to be sorted out (primarily with
UX). Mainly about how we handle scheduled items, which already have time
information. For now, it looks like this:
<img width="426" height="394" alt="image"
src="https://github.com/user-attachments/assets/4bfc4ca2-c738-4954-9251-8d063143371e"
/>
<img width="700" height="246" alt="image"
src="https://github.com/user-attachments/assets/fe688b08-c5c8-40f8-a9d0-fe455e44665f"
/>
The console was complaining. I suspect it was because of the wrapping
fragment. So instead of doing everything within react, I switched to
using a standard case statement.
Also: because name is optional and not guaranteed to be unique, let's
use id for the key instead.
This PR updates how we show old/new strategy/segment names in change
requests, and also removes the name of the strategy type from the
change.
For the old/new names: instead of showing them stacked vertically, we
show them side by side (old name first, then new name).
Compare before:
<img width="967" alt="image"
src="https://github.com/user-attachments/assets/d3e36f49-4abc-4cd4-8ba9-752515740185"
/>
with after:
<img width="974" alt="image"
src="https://github.com/user-attachments/assets/d0f85264-b055-4c44-b985-f992f09d8dab"
/>
Only affects the new components (legacy CR view is untouched). If we get
negative feedback while rolling it out because the strat type name is
missing, we can always add it back.
The max width is set to `max(40vw, 1000px)`, so that if you're on a very
wide window, then it'll take up at most 40% of the horizontal space.
Once your window is smaller than 2500px, however, the sidebar will stop
shrinking and stay at 1000px (or as close to that as the window allows).
It'll keep shrinking with the window size.
This came up because in certain cases, such as if you have a release
template with a long description, the modal would just keep growing
until it took up 98% of the window width.
I have not set a min width for now. I don't think there is any need for
it, but if we find there is, we can add it back later.
Before:
After:
Adds "change:" to the beginning of all changes and does some work to
align the use of compononents and structure across them (supersedes
https://github.com/Unleash/unleash/pull/10260).
In doing so, I have also added new and legacy variants for all different
change components, because this has required some hierarchy
restructuring every now and then. A reason for doing that was adding the
correct wrapping behavior for components, such that on smaller screens,
we wouldn't entirely blow out and make the kebab menu invisible and
inaccessible.
It also makes it so that we switch to full-width change view earlier (at
breakpoint md instead of sm), because at sm, a lot of stuff got hidden
before we switched to full-width.
Most changes are trivial updates; I've called out bits of the code that
are not in comments.
Rendered, it looks like this:
<img width="1203" alt="image"
src="https://github.com/user-attachments/assets/36bed974-99da-4d8d-a881-ea9df7797210"
/>
One interesting and potentially quite useful side-effect, is that all
change types now use the exact same set of components in the same
fashion, as evidenced by this screenie where I've added outlines to the
hierarchy:
<img width="1020" alt="image"
src="https://github.com/user-attachments/assets/685fefcc-af7e-4697-b8f3-8260af1e2a84"
/>
The one difference is that components without a diff place the "more"
kebab menu one layer further inside to facilitate prettier wrapping (the
kebab menu can stay on the same line as the other text when wrapping):
<img width="238" alt="image"
src="https://github.com/user-attachments/assets/2b8d3174-06a8-4ad4-b366-cea97720deda"
/>
Removes the view diff hover links (and strategy icons) in the new views
and removes trailing colons.
In removing the hover links, I have split up the content of their files
(`StrategyTooltipLink` and `SegmentTooltipLink`) into
`Change{Segment|Strategy}Name` and `{Segment|Strategy}Diff`. I have
reverted the existing tooltip files to their state before I began
changing this and added deprecation notices. These old tooltips are only
used by the old components, so we don't need to work on them after all.
In doing this work, I've also updated the strategy change diff to handle
the new functionality (tabs instead of hover)
The removal of the trailing colons (so that it's `adding strategy
gradual rollout` instead of `adding strategy: gradual rollout` is in
preparation for the remaining changes to the header that we're
introducing with this change. The removal of these is not behind a flag,
so I've also done it in the legacy components. This feels like a very
low risk change, so it felt like more work to have to check a flag for
each of the different instances where we use it.
<img width="839" alt="image"
src="https://github.com/user-attachments/assets/b29b8073-c282-4b4b-948f-9a545082ac31"
/>
We don't want deleted strategies to have the red background color
anymore with the new change/diff view. As such, we can remove it in the
new component and add a todo to delete the css for it after it's gone.
<img width="1050" alt="image"
src="https://github.com/user-attachments/assets/083cbcac-5df9-47cd-bd78-2501c3bbf64e"
/>
Updates the strategy change component used in change requests to have a
"tab"-like functionality, where you can switch between "Change" (the
rendered strategy) and "View diff" (the JSON diff). This is a tab
switcher, so you navigate it with arrow keys if on a keyboard, and I've
set selection to follow focus for now. This is my preference as it saves
you extra space/enter taps, but we can remove it later if we want.
Most of the changes in this PR is wrapping the existing strategy change
components/sections in tab panels and tab context providers.
Later changes in this project will remove the existing "view diff" hover
link in favor of this view.
There is some work to do on the design front (in terms of spacing,
borders, etc), but this covers the core functionality.
The pre-existing strategy change component has been moved into the
"LegacyStrategyChange" file with no changes beyond a deprecation
comment. The existing tests now test the new component instead with no
breakage. (I anticipate breaking when we remove the view diff link,
though)
Change with strategy variants:
<img width="991" alt="image"
src="https://github.com/user-attachments/assets/ac28912f-5b08-4a9c-96da-81bfd0b2e68e"
/>
<img width="1005" alt="image"
src="https://github.com/user-attachments/assets/4addaacc-101c-46cb-888f-95dc3b1cac25"
/>
## Edge case: deleted strat with no reference strategy
There is a case in the code where "reference strategy" can be undefined
for a deleted strategy. It is defined as follows:
```ts
const referenceStrategy =
changeRequestState === 'Applied'
? change.payload.snapshot
: currentStrategy;
```
I've decided to still show the "(no changes)" json diff in that setting,
so that the tabs actually affect something. I don't know how often this
happens, though: probably not anymore unless your CR is _ancient_, as we
introduced the snapshot quite a while ago, and `currentStrategy`
shouldn't really be undefined here, I should think.
Deleted strategy with no reference strategy:
Change:
<img width="1013" alt="image"
src="https://github.com/user-attachments/assets/352eaec9-c0ef-4d5a-b765-11304daf4474"
/>
Diff:
<img width="1029" alt="image"
src="https://github.com/user-attachments/assets/e69c81a6-1ef7-47ff-853a-9fb900b26303"
/>
Adds a number of small corrections and fixes to (mainly) CR-related
component. Discovered as part of adding the new JSON diff view. Refer to
the various inline comments for explanations.
---------
Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
This PR continues the cleanup after removing the addEditStrategy flag
(part 2 of ???). The primary purpose of this PR is to delete and remove
all references to the LegacyConstraintAccordion.
I've gone and updated all references to the legacy files in external
components and verified manually that they still work.
Most of the files in this PR are changing references. I've extracted two
bits into more general constants/utils:
1. Constraint IDs are a symbol. it was exported as a const from the
previous createEmptyConstraint file. I've moved it into constants.
2. formatOperatorDescription was similarly used all over the place, so
I've placed it in the shared utils directory.
In reviewing this, you can ignore any changes in the legacy constraint
accordion folder, because that's all been deleted. Instead, focus on the
changes in the other files. It's primarily just import updates, but
would be good to get a second set of eyes, anyway.
Removes all usages of flag addEditStrategy and refactors code where
necessary.
This is only the first step of the cleanup. After this, there's still
lots of code to be removed. I've got a different PR that removes ~5k
lines of code (https://github.com/Unleash/unleash/pull/10105) that I
want to reach in pieces to make sure that everythnig works on the way
there.
This removes a strategy that was already deprecated, but only for new
installations.
I tested starting with an installation with this strategy being used and
then updating, and I was still able to edit the strategy, so this should
not impact current users.
On a fresh install the strategy is no longer available.
---------
Co-authored-by: Nuno Góis <github@nunogois.com>
We're migrating to ESM, which will allow us to import the latest
versions of our dependencies.
Co-Authored-By: Christopher Kolstad <chriswk@getunleash.io>
Code for constraint accordion was copy-pasted before previous
improvement. Old version is still in use for Segments. When we get to
improving constraint editing we should rebuild segments editing, without
use of this code.
- 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
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
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.
