[Chrome Next] Migrate spaces to AppHeader - #288216
Conversation
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
There was a problem hiding this comment.
Header, badges and tabs all behave correctly in manual testing. Three things:
-
Spinner never shows while the space itself is loading. The
space &&conjunct inspace && !isPageReadymeans that while space is undefined you get the fallback title over an empty body; the spinner only appears afterwards, during the features/roles fetch. Droppingspace &&should do it. -
No coverage for the new header content. Nothing asserts the solution/current badges, the tab set, or the Permissions role-count badge.
-
The space avatar is removed from the header here as well (user avatar is also removed in user PR). In this case it may not be a concern since the avatar is rendered and editable in the body — unlike the user avatar in #288023, which is no longer rendered at all.
🤖 AI assisted review
|
Dropped the Added EditSpace tests for the header: solution + Current badges, the General settings / Permissions / Content tabs, and the Permissions role-count badge. Left the space avatar in the body only - it’s still rendered and editable there. |
jeramysoucy
left a comment
There was a problem hiding this comment.
Thanks for the update! The previous items are all addressed. Sorry, found a few things, mostly nits, but worth looking at before you merge.
| const badges: AppHeaderBadge[] = []; | ||
| if (shouldShowSolutionBadge) { | ||
| badges.push({ | ||
| label: solution, |
There was a problem hiding this comment.
Nit: label: solution is the raw enum value. It's inert today since SpaceSolutionBadge derives its own translated label and ignores badgeText. But renderCustomBadge is @deprecated and carries a TODO to remove custom JSX badges, so whoever does that cleanup will fall back to label and ship "oblt" instead of "Observability". Passing the translated solution name costs nothing and removes the trap.
There was a problem hiding this comment.
Addressed in: 533b95e
And just FYI, while renderCustomBadge is indeed deprecated, there are no plans to actually remove it anytime soon as this would require aligning with current consumers (the API was introduced so we could migrate DnD ASAP).
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Page load bundle
Unknown metric groupsshared async chunks total size
total optimizer output size
Test Failures
History
|
Summary
This PR migrates
Spacespages to the newAppHeader.Part of: https://github.com/elastic/kibana-team/issues/3945
Visuals
Classic
Before
spaces_classic_before.mov
After
spaces_classic_after.mov
Solution
Before
spaces_solution_before.mov
After
spaces_solution_after.mov