Replace `style` with `emotion.css` #149246

JordanSh · 2023-01-19T16:57:38Z

Many of our components use the React style native property to add styles.

We would like to replace as many of those as we can with @emotion/react

Attaching GH Thread to reference the motivations for the proposed changes

The text was updated successfully, but these errors were encountered:

elasticmachine · 2023-01-19T16:57:41Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

ArchitGajjar · 2024-01-03T23:43:22Z

Hi @Omolola-Akinleye @JordanSh - I would like to also help here. can you please share list of files which needs to replace styles with emotions/react ?

albertoblaz · 2024-12-02T12:17:37Z

My initial approach was too ambitious and wanted to do this change for the whole repo. That's why I started replacing all static inline styles in the src/ folder with this PR:

Global folders
- ✅ src/ Replace inline styles with css prop in src/ folder #201563

The problem is it requires approvals from many teams before merging, slowing down the overall progress. After looking at CODEOWNERS, I shifted to do this change only in the parts our team is responsible of. This is the work we need to do:

Packages we own:
- ✅ x-pack/packages/kbn-cloud-security-posture: Replace style with css prop in CSP package #202013
- ✅ x-pack/packages/security-solution/distribution_bar: no required changes
Plugins we own:
- ✅ x-pack/plugins/asset_inventory: no required changes
- ✅ x-pack/plugins/cloud_defend: Replace style with css prop in Cloud Defend plugin #202449
- 🔍 x-pack/plugins/cloud_security_posture Replace style with css in CSP plugin #202487
- ✅ x-pack/plugins/kubernetes_security: no required changes
- 🔍 x-pack/plugins/session_view: Replace style with css prop in SessionView plugin #202778
- ❌ x-pack/plugins/security_solution
Plugins we co-own:
- ❌ x-pack/plugins/fleet

## Summary Part of the resolution of this issue: - #149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### How to review From [Emotion's official docs](https://emotion.sh/docs/css-prop#style-precedence): > [!NOTE] > Any component or element that accepts a `className` prop can also use the `css` prop. The styles supplied to the `css` prop are evaluated and the computed class name is applied to the `className` prop. Components that are safe to migrate from `style` to `css`: - React elements - React components exposed by EUI (they all support a `className` prop and the [Babel plugin](https://www.npmjs.com/package/@emotion/babel-preset-css-prop) is enabled throughout Kibana) Components where styling might break: - Custom component we wrote that currently don't accept a `className` prop. - Specific 3P components that pass in a `style` prop to our components i.e. `react-window` (it calculates the dynamic position of all virtualized elements and hides the ones outside the visible fold) - Specific 3P components that expect a `style` prop to be styled i.e. charts, etc. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Main risk is breaking the UI by not applying or incorrectly applying the styling. This was explained in the "How to review" section above. The risk has **low severity** though. The mitigation plan will simply be reverting the commit asap not to affect production and test locally until it's fixed./

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### How to review From [Emotion's official docs](https://emotion.sh/docs/css-prop#style-precedence): > [!NOTE] > Any component or element that accepts a `className` prop can also use the `css` prop. The styles supplied to the `css` prop are evaluated and the computed class name is applied to the `className` prop. Components that are safe to migrate from `style` to `css`: - React elements - React components exposed by EUI (they all support a `className` prop and the [Babel plugin](https://www.npmjs.com/package/@emotion/babel-preset-css-prop) is enabled throughout Kibana) Components where styling might break: - Custom component we wrote that currently don't accept a `className` prop. - Specific 3P components that pass in a `style` prop to our components i.e. `react-window` (it calculates the dynamic position of all virtualized elements and hides the ones outside the visible fold) - Specific 3P components that expect a `style` prop to be styled i.e. charts, etc. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Main risk is breaking the UI by not applying or incorrectly applying the styling. This was explained in the "How to review" section above. The risk has **low severity** though. The mitigation plan will simply be reverting the commit asap not to affect production and test locally until it's fixed./ (cherry picked from commit 6aaecd0)

## Summary Part of the resolution of this issue: - #149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Screenshots <details><summary>Default Edge</summary> <img width="1028" alt="Screenshot 2024-12-02 at 16 27 47" src="https://github.com/user-attachments/assets/4c913a69-ee26-4cda-829c-2b26799ead81"> </details> <details><summary>Graph Popovers</summary> <img width="175" alt="Screenshot 2024-12-02 at 16 27 57" src="https://github.com/user-attachments/assets/55054b05-9cb4-4ca7-a19a-319277d7961d"> </details> <details><summary>Graph Stacked Edge Cases</summary> <img width="1319" alt="Screenshot 2024-12-02 at 16 28 03" src="https://github.com/user-attachments/assets/11ec7a03-e8cf-4090-9443-56288bf78a2c"> </details> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks - Minor risk with low impact and severity: - Only risk is CSP graph nodes showing with a different background and border

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Screenshots <details><summary>Default Edge</summary> <img width="1028" alt="Screenshot 2024-12-02 at 16 27 47" src="https://github.com/user-attachments/assets/4c913a69-ee26-4cda-829c-2b26799ead81"> </details> <details><summary>Graph Popovers</summary> <img width="175" alt="Screenshot 2024-12-02 at 16 27 57" src="https://github.com/user-attachments/assets/55054b05-9cb4-4ca7-a19a-319277d7961d"> </details> <details><summary>Graph Stacked Edge Cases</summary> <img width="1319" alt="Screenshot 2024-12-02 at 16 28 03" src="https://github.com/user-attachments/assets/11ec7a03-e8cf-4090-9443-56288bf78a2c"> </details> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks - Minor risk with low impact and severity: - Only risk is CSP graph nodes showing with a different background and border (cherry picked from commit a32d9c7)

opauloh · 2024-12-03T00:52:07Z

My initial approach was too ambitious and wanted to do this change for the whole repo.

The initiative was great! But splitting it really sounds like the way to go in this case

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Screenshots <details><summary>Default Edge</summary> <img width="1028" alt="Screenshot 2024-12-02 at 16 27 47" src="https://github.com/user-attachments/assets/4c913a69-ee26-4cda-829c-2b26799ead81"> </details> <details><summary>Graph Popovers</summary> <img width="175" alt="Screenshot 2024-12-02 at 16 27 57" src="https://github.com/user-attachments/assets/55054b05-9cb4-4ca7-a19a-319277d7961d"> </details> <details><summary>Graph Stacked Edge Cases</summary> <img width="1319" alt="Screenshot 2024-12-02 at 16 28 03" src="https://github.com/user-attachments/assets/11ec7a03-e8cf-4090-9443-56288bf78a2c"> </details> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks - Minor risk with low impact and severity: - Only risk is CSP graph nodes showing with a different background and border

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### How to review From [Emotion's official docs](https://emotion.sh/docs/css-prop#style-precedence): > [!NOTE] > Any component or element that accepts a `className` prop can also use the `css` prop. The styles supplied to the `css` prop are evaluated and the computed class name is applied to the `className` prop. Components that are safe to migrate from `style` to `css`: - React elements - React components exposed by EUI (they all support a `className` prop and the [Babel plugin](https://www.npmjs.com/package/@emotion/babel-preset-css-prop) is enabled throughout Kibana) Components where styling might break: - Custom component we wrote that currently don't accept a `className` prop. - Specific 3P components that pass in a `style` prop to our components i.e. `react-window` (it calculates the dynamic position of all virtualized elements and hides the ones outside the visible fold) - Specific 3P components that expect a `style` prop to be styled i.e. charts, etc. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Main risk is breaking the UI by not applying or incorrectly applying the styling. This was explained in the "How to review" section above. The risk has **low severity** though. The mitigation plan will simply be reverting the commit asap not to affect production and test locally until it's fixed./

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Screenshots <details><summary>Default Edge</summary> <img width="1028" alt="Screenshot 2024-12-02 at 16 27 47" src="https://github.com/user-attachments/assets/4c913a69-ee26-4cda-829c-2b26799ead81"> </details> <details><summary>Graph Popovers</summary> <img width="175" alt="Screenshot 2024-12-02 at 16 27 57" src="https://github.com/user-attachments/assets/55054b05-9cb4-4ca7-a19a-319277d7961d"> </details> <details><summary>Graph Stacked Edge Cases</summary> <img width="1319" alt="Screenshot 2024-12-02 at 16 28 03" src="https://github.com/user-attachments/assets/11ec7a03-e8cf-4090-9443-56288bf78a2c"> </details> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks - Minor risk with low impact and severity: - Only risk is CSP graph nodes showing with a different background and border

## Summary Part of the resolution of this issue: - #149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: Elastic Machine <[email protected]>

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit c1addc9)

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### How to review From [Emotion's official docs](https://emotion.sh/docs/css-prop#style-precedence): > [!NOTE] > Any component or element that accepts a `className` prop can also use the `css` prop. The styles supplied to the `css` prop are evaluated and the computed class name is applied to the `className` prop. Components that are safe to migrate from `style` to `css`: - React elements - React components exposed by EUI (they all support a `className` prop and the [Babel plugin](https://www.npmjs.com/package/@emotion/babel-preset-css-prop) is enabled throughout Kibana) Components where styling might break: - Custom component we wrote that currently don't accept a `className` prop. - Specific 3P components that pass in a `style` prop to our components i.e. `react-window` (it calculates the dynamic position of all virtualized elements and hides the ones outside the visible fold) - Specific 3P components that expect a `style` prop to be styled i.e. charts, etc. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Main risk is breaking the UI by not applying or incorrectly applying the styling. This was explained in the "How to review" section above. The risk has **low severity** though. The mitigation plan will simply be reverting the commit asap not to affect production and test locally until it's fixed./

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Screenshots <details><summary>Default Edge</summary> <img width="1028" alt="Screenshot 2024-12-02 at 16 27 47" src="https://github.com/user-attachments/assets/4c913a69-ee26-4cda-829c-2b26799ead81"> </details> <details><summary>Graph Popovers</summary> <img width="175" alt="Screenshot 2024-12-02 at 16 27 57" src="https://github.com/user-attachments/assets/55054b05-9cb4-4ca7-a19a-319277d7961d"> </details> <details><summary>Graph Stacked Edge Cases</summary> <img width="1319" alt="Screenshot 2024-12-02 at 16 28 03" src="https://github.com/user-attachments/assets/11ec7a03-e8cf-4090-9443-56288bf78a2c"> </details> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks - Minor risk with low impact and severity: - Only risk is CSP graph nodes showing with a different background and border

## Summary Part of the resolution of this issue: - elastic#149246 Removes the `style` prop in React components and elements to avoid using inline styles. Instead, it uses now the `emotion.css` prop to dynamically attach all styles to the native `class` attribute. ### Motivation Using inline styles at scale causes a performance penalty at rendering time. It's way more efficient to attach styles to a single or several classnames instead. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: Elastic Machine <[email protected]>

JordanSh added the Team:Cloud Security Cloud Security team related label Jan 19, 2023

opauloh added the performance label Jan 23, 2023

JordanSh mentioned this issue Jan 29, 2023

[Cloud Posture] [Dashboard] Add sorting by Compliance Score in the clusters section #149566

Merged

kfirpeled added the 8.9 candidate label Apr 17, 2023

kfirpeled assigned JordanSh Apr 23, 2023

kfirpeled added the technical debt Improvement of the software architecture and operational architecture label May 2, 2023

kfirpeled added 8.10 candidate and removed 8.9 candidate labels May 22, 2023

kfirpeled unassigned JordanSh Jun 11, 2023

tehilashn added the good first issue low hanging fruit label Jul 17, 2023

kfirpeled changed the title ~~[Cloud Posture] Replace style with emotion.css~~ [Cloud Security] Replace style with emotion.css Aug 9, 2023

kfirpeled added 8.11 candidate and removed 8.10 candidate labels Aug 9, 2023

kfirpeled assigned JordanSh Aug 17, 2023

kfirpeled removed the 8.11 candidate label Sep 5, 2023

kfirpeled unassigned JordanSh Nov 28, 2023

Omolola-Akinleye self-assigned this Nov 29, 2023

Omolola-Akinleye added the 8.13 candidate label Nov 29, 2023

kfirpeled removed the 8.13 candidate label Jan 28, 2024

kfirpeled unassigned Omolola-Akinleye Jan 31, 2024

acorretti changed the title ~~[Cloud Security] Replace style with emotion.css~~ Replace style with emotion.css Sep 19, 2024

albertoblaz self-assigned this Nov 21, 2024

This was referenced Nov 25, 2024

Replace inline styles with css prop in src/ folder #201563

Merged

Replace style with css prop in CSP package #202013

Merged

albertoblaz mentioned this issue Dec 2, 2024

Replace style with css prop in Cloud Defend plugin #202449

Merged

1 task

albertoblaz mentioned this issue Dec 2, 2024

Replace style with css in CSP plugin #202487

Open

1 task

albertoblaz mentioned this issue Dec 3, 2024

Replace style with css prop in SessionView plugin #202778

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `style` with `emotion.css` #149246

Replace `style` with `emotion.css` #149246

JordanSh commented Jan 19, 2023 •

edited by opauloh

Loading

elasticmachine commented Jan 19, 2023

ArchitGajjar commented Jan 3, 2024

albertoblaz commented Dec 2, 2024 •

edited

Loading

opauloh commented Dec 3, 2024

Replace style with emotion.css #149246

Replace style with emotion.css #149246

Comments

JordanSh commented Jan 19, 2023 • edited by opauloh Loading

elasticmachine commented Jan 19, 2023

ArchitGajjar commented Jan 3, 2024

albertoblaz commented Dec 2, 2024 • edited Loading

opauloh commented Dec 3, 2024

Replace `style` with `emotion.css` #149246

Replace `style` with `emotion.css` #149246

JordanSh commented Jan 19, 2023 •

edited by opauloh

Loading

albertoblaz commented Dec 2, 2024 •

edited

Loading