Adds filter buttons for filtering between "CRs created by me" and "CRs
where I've been requested as an approver".
The current implementation is fairly simplistic and the buttons are not
connected to the actual table state directly (instead being set up with
their own simple state and onChange hooks), but it covers the simple
scenario. I want to defer a more complex solution until we know we need
it and until we know exactly what we need. The implementation is based
on the lifecycle filters that we have on the project flags page.
The current logic is such that: when you land on the page, there's no
query params in the URL, but the data fetch applies `createdBy:IS<your
user>`. If you switch to "approval requested" (and back again), the URL
will reflect this.
For reference, the github workflow works like this, where each URL has a
set of default filters, e.g.:
- `/pulls`: `is:open is:pr assignee:thomasheartman archived:false`
- `/pulls/review-requested`: `is:open is:pr
review-requested:thomasheartman archived:false`
But if you change the default filters or add new ones, the URL will
update to `pulls?<query-string>` (e.g.
`/pulls?q=is%3Aopen+is%3Apr+review-requested%3Athomasheartman+archived%3Atrue`)
So this takes a similar approach, but better suited to the way we do
tables in general.
Rendered:
<img width="1816" height="791" alt="image"
src="https://github.com/user-attachments/assets/60935900-488d-4ca9-b110-39f3568a08a6"
/>
<img width="1855" height="329" alt="image"
src="https://github.com/user-attachments/assets/5e865a2e-8fdc-41ab-ba38-bbe6776d04ad"
/>
Adds a paginated table to the change request overview page and
integrates it with the search API hook.
The current implementation still has some rough edges to work out, but
it's getting closer.
There's no sort buttons in this implementation. I've got it working on
the side, but TS is complaining about types not matching up, so I'm
spinning that out to a separate PR.
<img width="1808" height="1400" alt="image"
src="https://github.com/user-attachments/assets/bdee97b7-ee2a-46c0-8460-a8b8e14d3c92"
/>
Adds basic table layout for the global change requests page and makes
the page accessible at `/change-requests`.
The table is based on the project-based change request table, but with a
slightly different set of columns.
Uses mock data for now.
There's still some styling to be done for the column widths and handling
narrower screens.
<img width="1386" height="671" alt="image"
src="https://github.com/user-attachments/assets/b24ed625-d3f6-4281-ba44-30744d5063f3"
/>
If the flag is disabled, we render nothing useful.
<img width="1429" height="287" alt="image"
src="https://github.com/user-attachments/assets/289b5707-4389-4c08-bf68-55d63e186ba5"
/>
closes 1-4076
This PR cleans up the crDiffView flag. These changes were automatically
generated by AI and should be reviewed carefully.
Fixes#10484🧹 AI Flag Cleanup Summary
This PR removes the crDiffView feature flag and its associated legacy
components
for displaying changes in a Change Request. The flag has been enabled
and the
new diff view is now permanent.
This involved removing the feature flag from the configuration and code,
deleting several legacy components, and updating the components that
used them
to only use the new versions.
🚮 Removed
• Feature Flag Logic
• All checks for the crDiffView flag.
• The flag definition in uiConfig.ts, experimental.ts, and
server-dev.ts.
• Legacy Components
• LegacyStrategyChange.tsx
• StrategyTooltipLink.tsx
• LegacyReleasePlanChange.tsx
• SegmentTooltipLink.tsx
• LegacySegmentChangeDetails.tsx
• LegacyArchiveFeatureChange from ArchiveFeatureChange.tsx
• LegacyDependencyChange from DependencyChange.tsx
• LegacyToggleStatusChange from ToggleStatusChange.tsx
🛠 Kept
• New Components
• The new change request diff view components (StrategyChange,
ReleasePlanChange, etc.) are now used directly.
• The UI for displaying changes in a Change Request now consistently
uses
the improved diff view.
📝 Why
The crDiffView feature flag was deemed complete and ready for permanent
implementation. The cleanup follows standard procedure to remove the
flag and
associated dead code, simplifying the codebase and making it easier to
maintain.
This change makes the improved diff view for change requests the only
available
view.
---------
Co-authored-by: unleash-bot <194219037+unleash-bot[bot]@users.noreply.github.com>
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
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>
