-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[Fleet][Eui Visual Refresh] Update text color tokens #204537
[Fleet][Eui Visual Refresh] Update text color tokens #204537
Conversation
Pinging @elastic/fleet (Team:Fleet) |
@jillguyonnet LGTM 💚 |
<EuiContextMenuItem | ||
{...props} | ||
css={css` | ||
color: ${theme.euiTheme.colors.textDanger}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking cleanup suggestion:
This is perfectly fine, but you can also use the object notation, which removes the need for importing css
and using useEuiTheme
specifically (since it's only used for this one case).
css={({ euiTheme }) => ({
color: euiTheme.colors.textDanger,
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mgadewoll thanks - I had tried that but ran into
TypeError: Cannot read properties of undefined (reading 'colors')
Here's the full component:
import React from 'react';
import type { EuiContextMenuItemProps } from '@elastic/eui';
import { EuiContextMenuItem } from '@elastic/eui';
export const DangerEuiContextMenuItem = (props: EuiContextMenuItemProps) => {
return (
<EuiContextMenuItem
{...props}
css={({ euiTheme }) => ({
color: euiTheme.colors.textDanger,
})}
/>
);
};
Did I miss anything?
I found a similar occurrence with a note that an error is expected, leading to elastic/eui#8123. This comment suggests that the issue should be resolved though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jillguyonnet Ah right, then your plugin doesn't have the required type set up yet.
You'll need to add types for your plugin to support this. (emotion docs)
Here an example of the required module.
And this then should be included in your tsconfig.json
(example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I'm capturing some frontend cleanup for Fleet UI in a dedicated task, I will add this in order to make getting the last EUI visual refresh PR over the line a priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, that makes perfect sense. It wasn't blocking for this PR 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💚 Build Succeeded
Metrics [docs]Async chunks
|
## Summary Closes elastic#201989 This PR updates color variables to the up-to-date naming per elastic#199715 (comment). Impacted elements: 1. Read-only icon buttons in Fleet and Integrations top bar. 2. Danger-styled context menu item for deleting package policy from agent policy. I have also reviews [all occurrences](https://github.com/search?q=repo%3Aelastic%2Fkibana+path%3Ax-pack%2Fplugins%2Ffleet%2Fpublic+color%3D%22text&type=code) of `color="text` throughout fleet and found these to be part of the following components: - `EuiButton` - `EuiButtonEmpty` - `EuiButtonIcon` - `EuiIcon` The use of `color="text"` seems to be valid for these cases. ## Screenshots ### Amsterdam Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-amsterdam" src="https://github.com/user-attachments/assets/c6c58a75-b79b-45a9-abef-f25014a7c8c6" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-amsterdam" src="https://github.com/user-attachments/assets/fa2a5b10-4aca-46ee-bb59-c0f62091c322" /> Delete integration context menu item: <img width="1918" alt="delete-integration-amsterdam" src="https://github.com/user-attachments/assets/c2375413-373b-4cdf-b907-0d5f577e1fbe" /> ### Borealis Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-borealis" src="https://github.com/user-attachments/assets/aad169c6-52b0-4707-87a5-1e0babd59a43" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-borealis" src="https://github.com/user-attachments/assets/08d1cec7-285d-47a3-958b-63b5a22d9b19" /> Delete integration context menu item: <img width="1918" alt="delete-integration-borealis" src="https://github.com/user-attachments/assets/9cf20708-2a3f-4252-98ac-1ebca12bd8e6" />
## Summary Closes elastic#201989 This PR updates color variables to the up-to-date naming per elastic#199715 (comment). Impacted elements: 1. Read-only icon buttons in Fleet and Integrations top bar. 2. Danger-styled context menu item for deleting package policy from agent policy. I have also reviews [all occurrences](https://github.com/search?q=repo%3Aelastic%2Fkibana+path%3Ax-pack%2Fplugins%2Ffleet%2Fpublic+color%3D%22text&type=code) of `color="text` throughout fleet and found these to be part of the following components: - `EuiButton` - `EuiButtonEmpty` - `EuiButtonIcon` - `EuiIcon` The use of `color="text"` seems to be valid for these cases. ## Screenshots ### Amsterdam Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-amsterdam" src="https://github.com/user-attachments/assets/c6c58a75-b79b-45a9-abef-f25014a7c8c6" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-amsterdam" src="https://github.com/user-attachments/assets/fa2a5b10-4aca-46ee-bb59-c0f62091c322" /> Delete integration context menu item: <img width="1918" alt="delete-integration-amsterdam" src="https://github.com/user-attachments/assets/c2375413-373b-4cdf-b907-0d5f577e1fbe" /> ### Borealis Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-borealis" src="https://github.com/user-attachments/assets/aad169c6-52b0-4707-87a5-1e0babd59a43" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-borealis" src="https://github.com/user-attachments/assets/08d1cec7-285d-47a3-958b-63b5a22d9b19" /> Delete integration context menu item: <img width="1918" alt="delete-integration-borealis" src="https://github.com/user-attachments/assets/9cf20708-2a3f-4252-98ac-1ebca12bd8e6" />
## Summary Closes elastic#201989 This PR updates color variables to the up-to-date naming per elastic#199715 (comment). Impacted elements: 1. Read-only icon buttons in Fleet and Integrations top bar. 2. Danger-styled context menu item for deleting package policy from agent policy. I have also reviews [all occurrences](https://github.com/search?q=repo%3Aelastic%2Fkibana+path%3Ax-pack%2Fplugins%2Ffleet%2Fpublic+color%3D%22text&type=code) of `color="text` throughout fleet and found these to be part of the following components: - `EuiButton` - `EuiButtonEmpty` - `EuiButtonIcon` - `EuiIcon` The use of `color="text"` seems to be valid for these cases. ## Screenshots ### Amsterdam Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-amsterdam" src="https://github.com/user-attachments/assets/c6c58a75-b79b-45a9-abef-f25014a7c8c6" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-amsterdam" src="https://github.com/user-attachments/assets/fa2a5b10-4aca-46ee-bb59-c0f62091c322" /> Delete integration context menu item: <img width="1918" alt="delete-integration-amsterdam" src="https://github.com/user-attachments/assets/c2375413-373b-4cdf-b907-0d5f577e1fbe" /> ### Borealis Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-borealis" src="https://github.com/user-attachments/assets/aad169c6-52b0-4707-87a5-1e0babd59a43" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-borealis" src="https://github.com/user-attachments/assets/08d1cec7-285d-47a3-958b-63b5a22d9b19" /> Delete integration context menu item: <img width="1918" alt="delete-integration-borealis" src="https://github.com/user-attachments/assets/9cf20708-2a3f-4252-98ac-1ebca12bd8e6" />
## Summary Closes elastic#201989 This PR updates color variables to the up-to-date naming per elastic#199715 (comment). Impacted elements: 1. Read-only icon buttons in Fleet and Integrations top bar. 2. Danger-styled context menu item for deleting package policy from agent policy. I have also reviews [all occurrences](https://github.com/search?q=repo%3Aelastic%2Fkibana+path%3Ax-pack%2Fplugins%2Ffleet%2Fpublic+color%3D%22text&type=code) of `color="text` throughout fleet and found these to be part of the following components: - `EuiButton` - `EuiButtonEmpty` - `EuiButtonIcon` - `EuiIcon` The use of `color="text"` seems to be valid for these cases. ## Screenshots ### Amsterdam Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-amsterdam" src="https://github.com/user-attachments/assets/c6c58a75-b79b-45a9-abef-f25014a7c8c6" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-amsterdam" src="https://github.com/user-attachments/assets/fa2a5b10-4aca-46ee-bb59-c0f62091c322" /> Delete integration context menu item: <img width="1918" alt="delete-integration-amsterdam" src="https://github.com/user-attachments/assets/c2375413-373b-4cdf-b907-0d5f577e1fbe" /> ### Borealis Read-only Fleet top bar: <img width="1918" alt="fleet-readonly-borealis" src="https://github.com/user-attachments/assets/aad169c6-52b0-4707-87a5-1e0babd59a43" /> Read-only Integrations top bar: <img width="1918" alt="integrations-readonly-borealis" src="https://github.com/user-attachments/assets/08d1cec7-285d-47a3-958b-63b5a22d9b19" /> Delete integration context menu item: <img width="1918" alt="delete-integration-borealis" src="https://github.com/user-attachments/assets/9cf20708-2a3f-4252-98ac-1ebca12bd8e6" />
Summary
Closes #201989
This PR updates color variables to the up-to-date naming per #199715 (comment).
Impacted elements:
I have also reviews all occurrences of
color="text
throughout fleet and found these to be part of the following components:EuiButton
EuiButtonEmpty
EuiButtonIcon
EuiIcon
The use of
color="text"
seems to be valid for these cases.Screenshots
Amsterdam
Read-only Fleet top bar:
Read-only Integrations top bar:
Delete integration context menu item:
Borealis
Read-only Fleet top bar:
Read-only Integrations top bar:
Delete integration context menu item: