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 SessionView plugin #202778

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

albertoblaz
Copy link
Contributor

@albertoblaz albertoblaz commented Dec 3, 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 3, 2024
@albertoblaz albertoblaz self-assigned this Dec 3, 2024
@albertoblaz albertoblaz requested a review from a team as a code owner December 3, 2024 17:22
@elasticmachine
Copy link
Contributor

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

@Omolola-Akinleye Omolola-Akinleye self-requested a review December 9, 2024 10:52
@Omolola-Akinleye
Copy link
Contributor

@albertoblaz Any screenshot to validate none of the styles broke?

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.

👍

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #57 / Actions and Triggers app Maintenance Windows Maintenance windows table paginates maintenance windows correctly

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
sessionView 355.2KB 355.3KB +69.0B

Page load bundle

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

id before after diff
sessionView 48.7KB 48.7KB -4.0B

History

cc @albertoblaz

Copy link
Contributor

@maxcold maxcold left a comment

Choose a reason for hiding this comment

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

tested in Session View, lgtm

@albertoblaz albertoblaz merged commit 0d4ccaf into elastic:main Jan 13, 2025
8 checks passed
@albertoblaz albertoblaz deleted the replace-style-session-view branch January 13, 2025 12:53
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 13, 2025
## 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: Maxim Kholod <[email protected]>
(cherry picked from commit 0d4ccaf)
@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 Jan 13, 2025
…iew plugin (#202778) (#206426)

# Backport

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

<!--- 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":"2025-01-13T12:53:31Z","message":"Replace
`style` with `css` prop in SessionView plugin (#202778)\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\n---------\r\n\r\nCo-authored-by:
Maxim Kholod
<[email protected]>","sha":"0d4ccaf7b2da0028841eec6678fd20351d6e3303","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 SessionView
plugin","number":202778,"url":"https://github.com/elastic/kibana/pull/202778","mergeCommit":{"message":"Replace
`style` with `css` prop in SessionView plugin (#202778)\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\n---------\r\n\r\nCo-authored-by:
Maxim Kholod
<[email protected]>","sha":"0d4ccaf7b2da0028841eec6678fd20351d6e3303"}},"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/202778","number":202778,"mergeCommit":{"message":"Replace
`style` with `css` prop in SessionView plugin (#202778)\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\n---------\r\n\r\nCo-authored-by:
Maxim Kholod
<[email protected]>","sha":"0d4ccaf7b2da0028841eec6678fd20351d6e3303"}},{"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]>
delanni pushed a commit to delanni/kibana that referenced this pull request Jan 13, 2025
## 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: Maxim Kholod <[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.

6 participants