Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Fleet][EUI Visual refresh] Remove custom colors from code and replace them with EUI color tokens #202003

Closed
3 tasks
Tracked by #202008
criamico opened this issue Nov 27, 2024 · 2 comments · Fixed by #204717
Closed
3 tasks
Tracked by #202008

[Fleet][EUI Visual refresh] Remove custom colors from code and replace them with EUI color tokens #202003

criamico opened this issue Nov 27, 2024 · 2 comments · Fixed by #204717
Assignees
@jillguyonnet
Labels
Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@criamico
Copy link
Contributor

criamico commented Nov 27, 2024 •
edited
Loading

Context

The EUI team is introducing a new theme called "Borealis". As part of this change we need to update some colors in Fleet.
See parent issue: #199715 - it also contains the instructions to run the new theme (see section Instructions for updating and QA)

Checklist

  • Replace any custom colors across Fleet with EUI color tokens, either in code or inside css styling. Details

⚠️ Some occurrences across Fleet:

Definitions of done

  • When testing the changes, test them both with the "old" Amsterdam theme and with the new Borealis theme.
  • Please label the PR with EUI Visual Refresh for cross-team visibility
@criamico criamico added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 27, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@criamico criamico mentioned this issue Nov 27, 2024
Open
9 tasks
@jillguyonnet
Copy link
Contributor

jillguyonnet commented Dec 17, 2024 •
edited
Loading

@justinkambic Could you please comment on the colour choice for

kibana/x-pack/plugins/fleet/public/applications/integrations/sections/epm/components/package_card.tsx

Line 50 in 87af12d

border-color: ${({ isquickstart }) => (isquickstart ? '#ba3d76' : null)};

which was introduced in #179573?

It is not clear to me which colour token is suitable here.

Edit: replaced with accent 👍 Apologies about the ping.

@jillguyonnet jillguyonnet mentioned this issue Dec 18, 2024
Merged
10 tasks
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this issue Dec 19, 2024
@jillguyonnet @JoseLuisGJ
[Fleet][EUI Visual Refresh] Replace hardcoded colors with tokens (ela…
941bcde 
…stic#204717)

## Summary

Closes elastic#202003

This PR replaces hardcoded custom colours with EUI tokens.

Found occurrences in Fleet plugin codebase:
- hex colours (`#[A-Fa-f0-9]{6}`)
- colour names (`red`, `green`...).

- [x] Agent activity flyout
   - [x] Multiple hardcoded colours changed to semantic tokens
- [x] Missing security requirements page
   - [x] Border colour
- [x] Multisteps integration installation
- [x] There was an old styling polyfill that looks like it can be
removed, the result is visually very similar. Note that [the proposal to
introduce numberless steps was rejected based on accessibility
concerns](elastic/eui#5836 (comment)).
- [x] Agent donut chart
   - [x] Legacy component no longer in use - removed
- [x] Quickstart package card styling
- [x] Card border colour (cf. implementation in
elastic#179573)

Not covered:
- [Test custom space in Cypress
test](https://github.com/elastic/kibana/blob/6cd2cd6cf83fcf18b7ab66ac3f38f11ab9d69f8a/x-pack/plugins/fleet/cypress/tasks/spaces.ts#L26)
- [Mock useEuiTheme in Jest
test](https://github.com/elastic/kibana/blob/6cd2cd6cf83fcf18b7ab66ac3f38f11ab9d69f8a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/components/installation_status.test.tsx#L26)
-
[Story](https://github.com/elastic/kibana/blob/6cd2cd6cf83fcf18b7ab66ac3f38f11ab9d69f8a/x-pack/plugins/fleet/public/applications/integrations/sections/epm/components/integration_preference.stories.tsx#L24)
- [Custom tag
colours](https://github.com/elastic/kibana/blob/6cd2cd6cf83fcf18b7ab66ac3f38f11ab9d69f8a/x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.ts#L36-L57):
note that these are defined in the backend.

## Screenshots

Note: I'm struggling to test the integrations quick start (tested the
colour by manually applying it on the normal Integrations page), any
advice on this would be welcome.

### Before (`main` branch), Amsterdam theme

Agent activity flyout, showing in progress and error activity items:
<img width="800" alt="agent-activity-1-amsterdam-main"
src="https://github.com/user-attachments/assets/853dd496-415d-46f0-b3e0-c8e1bd0de574"
/>

Agent activity flyout, showing a successful activity item:
<img width="800" alt="agent-activity-2-amsterdam-main"
src="https://github.com/user-attachments/assets/dfa16c8c-cb3d-49e1-857f-b4ed2f658ad0"
/>

Missing security requirements page:
<img width="1919" alt="es-requirements-amsterdam-main"
src="https://github.com/user-attachments/assets/c029aa06-9326-4489-8e12-e26516e612d4"
/>

Multisteps integration installation (step 1):
<img width="1919" alt="multisteps-1-amsterdam-main"
src="https://github.com/user-attachments/assets/942a8e4f-6ed0-4498-a192-34fb4a319e37"
/>

Multisteps integration installation (step 2):
<img width="1919" alt="multisteps-2-amsterdam-main"
src="https://github.com/user-attachments/assets/c1976c63-08cc-4a91-b8c0-8ae73cb4afda"
/>

For reference, the old agent donut chard component:

![old-fleet-ui](https://github.com/user-attachments/assets/6b309d8a-5417-45cb-bb6c-58d1d88009f5)

### After

#### Amsterdam

Agent activity flyout, showing in progress and error activity items:
<img width="800" alt="agent-activity-1-amsterdam-branch"
src="https://github.com/user-attachments/assets/6765c849-e4bd-4744-b108-2bdc3337ffa6"
/>

Agent activity flyout, showing a successful activity item:
<img width="800" alt="agent-activity-2-amsterdam-branch"
src="https://github.com/user-attachments/assets/29d7ef48-16f5-40f2-9686-2d1a7f8e5737"
/>

Missing security requirements page:
<img width="1918" alt="es-requirements-amsterdam-branch"
src="https://github.com/user-attachments/assets/372807ad-41cb-4945-8300-eaba15a40db4"
/>

Multisteps integration installation (step 1):
<img width="1918" alt="multisteps-1-amsterdam-branch"
src="https://github.com/user-attachments/assets/41c566f0-200b-4cd6-a579-cc50292f4a56"
/>

Multisteps integration installation (step 2):
<img width="1918" alt="multisteps-2-amsterdam-branch"
src="https://github.com/user-attachments/assets/647ef4a8-d190-4a84-8a8c-07973cd600f8"
/>

#### Borealis

Agent activity flyout, showing in progress and error activity items:
<img width="800" alt="agent-activity-1-borealis-branch"
src="https://github.com/user-attachments/assets/1e9cbd66-82de-48ca-9550-e226c808c3e5"
/>

Agent activity flyout, showing a successful activity item:
<img width="800" alt="agent-activity-2-borealis-branch"
src="https://github.com/user-attachments/assets/690792f5-8c8f-4665-ba2d-72a087be8bae"
/>

Missing security requirements page:
<img width="1918" alt="es-requirements-borealis-branch"
src="https://github.com/user-attachments/assets/a65bd707-2374-440b-9730-b48e8730edca"
/>

Multisteps integration installation (step 1):
<img width="1918" alt="multisteps-1-borealis-branch"
src="https://github.com/user-attachments/assets/ef0e8b99-3bf5-4da1-8b72-731741c6d322"
/>

Multisteps integration installation (step 2):
<img width="1918" alt="multisteps-2-borealis-branch"
src="https://github.com/user-attachments/assets/a4901126-7686-420e-b13d-9f0c63aa71c9"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jillguyonnet jillguyonnet

Labels
Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fleet][EUI Visual Refresh] Replace hardcoded colors with tokens jillguyonnet/kibana
3 participants
@elasticmachine @criamico @jillguyonnet