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.

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

Replace style with css prop in Cloud Defend plugin #202449

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

albertoblaz
Copy link
Contributor

@albertoblaz albertoblaz commented Dec 2, 2024

Summary

Part of the resolution of this issue:

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.

  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

@albertoblaz albertoblaz added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) backport:version Backport to applied version labels v8.18.0 labels Dec 2, 2024
@albertoblaz albertoblaz self-assigned this Dec 2, 2024
@albertoblaz albertoblaz requested a review from a team as a code owner December 2, 2024 12:12
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@seanrathier seanrathier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ezee peezee! :)

@albertoblaz
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #7 / Visualizations - Group 2 lens app - Agg based Vis Open in Lens Table should convert percentage column

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cloudDefend 101.1KB 101.2KB +98.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
cloudDefend 8.9KB 9.0KB +25.0B

cc @albertoblaz

@albertoblaz albertoblaz merged commit c1addc9 into elastic:main Dec 11, 2024
8 checks passed
@albertoblaz albertoblaz deleted the replace-style-cloud-defend branch December 11, 2024 14:31
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12278514328

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 11, 2024
## 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)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 11, 2024
…fend plugin (#202449) (#203809)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Replace &#x60;style&#x60; with &#x60;css&#x60; prop in Cloud Defend
plugin (#202449)](#202449)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alberto
Blázquez","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-11T14:31:30Z","message":"Replace
`style` with `css` prop in Cloud Defend plugin (#202449)\n\n##
Summary\r\n\r\nPart of the resolution of this issue: \r\n-
https://github.com/elastic/kibana/issues/149246\r\n\r\nRemoves the
`style` prop in React components and elements to avoid using\r\ninline
styles. Instead, it uses now the `emotion.css` prop to\r\ndynamically
attach all styles to the native `class` attribute.\r\n\r\n###
Motivation\r\n\r\nUsing inline styles at scale causes a performance
penalty at rendering\r\ntime. It's way more efficient to attach styles
to a single or several\r\nclassnames instead.\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [x] The PR description includes the appropriate Release
Notes section,\r\nand the correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"c1addc9ff78c4b5bb97938eee257e3ba0a200109","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Cloud
Security","backport:prev-minor","backport:version","v8.18.0"],"title":"Replace
`style` with `css` prop in Cloud Defend
plugin","number":202449,"url":"https://github.com/elastic/kibana/pull/202449","mergeCommit":{"message":"Replace
`style` with `css` prop in Cloud Defend plugin (#202449)\n\n##
Summary\r\n\r\nPart of the resolution of this issue: \r\n-
https://github.com/elastic/kibana/issues/149246\r\n\r\nRemoves the
`style` prop in React components and elements to avoid using\r\ninline
styles. Instead, it uses now the `emotion.css` prop to\r\ndynamically
attach all styles to the native `class` attribute.\r\n\r\n###
Motivation\r\n\r\nUsing inline styles at scale causes a performance
penalty at rendering\r\ntime. It's way more efficient to attach styles
to a single or several\r\nclassnames instead.\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [x] The PR description includes the appropriate Release
Notes section,\r\nand the correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"c1addc9ff78c4b5bb97938eee257e3ba0a200109"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202449","number":202449,"mergeCommit":{"message":"Replace
`style` with `css` prop in Cloud Defend plugin (#202449)\n\n##
Summary\r\n\r\nPart of the resolution of this issue: \r\n-
https://github.com/elastic/kibana/issues/149246\r\n\r\nRemoves the
`style` prop in React components and elements to avoid using\r\ninline
styles. Instead, it uses now the `emotion.css` prop to\r\ndynamically
attach all styles to the native `class` attribute.\r\n\r\n###
Motivation\r\n\r\nUsing inline styles at scale causes a performance
penalty at rendering\r\ntime. It's way more efficient to attach styles
to a single or several\r\nclassnames instead.\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [x] The PR description includes the appropriate Release
Notes section,\r\nand the correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"c1addc9ff78c4b5bb97938eee257e3ba0a200109"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Alberto Blázquez <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants