chore(slo): remove deprecated theme provider and usage of styled-components #200248

kdelemme · 2024-11-14T19:37:12Z

Resolves #200247

🖌️ Summary

This PR removes all usage of styled-components from the SLO plugin, replacing them with @emotions/react. It also removes the deprecated Eui ThemeProvider.

🍵 Testing

Check all pages for UI glitches
Check all embeddables for UI glitches

Screenshots	Screenshots

elasticmachine · 2024-11-14T19:37:46Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

github-actions · 2024-11-14T19:56:40Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

kdelemme · 2024-11-18T18:07:43Z

x-pack/plugins/observability_solution/slo/public/application.tsx

@@ -116,15 +114,13 @@ export const renderApp = ({
                }}
              >
                <Router history={history}>
-                  <EuiThemeProvider darkMode={isDarkMode}>


removing this deprecated theme provider, kept <KibanaThemeProvider {...{ theme: { theme$ } }}> from above

cee-chen

I left some optional suggestions around using EUI size tokens (rather than hardcoded px values), as well as taking advantage of Emotion's css theme callbacks, but they're mostly optional. Overall this looks solid and I love how much code it cleans up!

cee-chen · 2024-11-18T18:47:09Z

x-pack/plugins/observability_solution/slo/public/embeddable/slo/burn_rate/burn_rate.tsx

@@ -84,7 +74,7 @@ export function BurnRate({ sloId, sloInstanceId, duration, reloadSubject }: Embe
          <EuiFlexItem>
            <EuiLink
              data-test-subj="sloBurnRateLink"
-              className={link}
+              css={link}


It would be nice to use EUI variables for the link CSS defined at the bottom of the page here instead of hard coded values. Something like:

const link = ({ euiTheme }: UseEuiTheme) => css` font-size: ${euiTheme.size.base}; font-weight: ${euiTheme.font.weight.bold}; `;

cee-chen · 2024-11-18T18:50:25Z

...plugins/observability_solution/slo/public/embeddable/slo/overview/slo_embeddable_factory.tsx

+                  css={css`
+                    width: 100%;
+                    padding: 5px 15px;
+                    overflow: scroll;
+
+                    .euiAccordion__buttonContent {
+                      min-width: 100px;
+                    }
+                  `}


Could we opt to use EUI token sizes here instead? The translations won't be exact but IMO it's probably good enough?

Suggested change

css={css`

width: 100%;

padding: 5px 15px;

overflow: scroll;

.euiAccordion__buttonContent {

min-width: 100px;

}

`}

css={({ euiTheme}) => css`

width: 100%;

padding: ${euiTheme.size.xs} ${euiTheme.size.base};

overflow: scroll;

.euiAccordion__buttonContent {

min-width: ${euiTheme.base * 6}px;

}

`}

cee-chen · 2024-11-18T18:51:32Z

...plugins/observability_solution/slo/public/embeddable/slo/overview/slo_embeddable_factory.tsx

+                      css={css`
                        margin-top: 20px;
                      `}


see above comment re: using EUI tokens

Suggested change

css={css`

margin-top: 20px;

`}

css={({ euiTheme }) => css`

margin-top: ${euiTheme.base * 1.25}px;

`}

cee-chen · 2024-11-18T18:52:55Z

...s/observability_solution/slo/public/pages/slos/components/card_view/slo_card_item_badges.tsx

+      css={css`
+        display: inline-block;
+        margin-top: 5px;
+      `}


Suggested change

css={css`

display: inline-block;

margin-top: 5px;

`}

css={({ euiTheme }) => css`

display: inline-block;

margin-top: ${euiTheme.size.xs};

`}

cee-chen · 2024-11-18T18:57:59Z

.../observability_solution/slo/public/pages/slos/components/card_view/slo_card_item_actions.tsx

+  const borderRadius = useEuiTheme().euiTheme.border.radius.medium;

  return (
-    <Container boxShadow={euiShadow} position={'relative'}>
+    <div
+      css={css`
+        display: inline-block;
+        position: relative;
+        bottom: 42px;
+        left: 12px;
+        z-index: 1;
+        border-radius: ${borderRadius};
+        ${useEuiShadow('l')}
+      `}
+    >


[optional] You can skip line 28 by using the Emotion's css theme callback:

Suggested change

const borderRadius = useEuiTheme().euiTheme.border.radius.medium;

return (

<Container boxShadow={euiShadow} position={'relative'}>

<div

css={css`

display: inline-block;

position: relative;

bottom: 42px;

left: 12px;

z-index: 1;

border-radius: ${borderRadius};

${useEuiShadow('l')}

`}

>

return (

<div

css={({ euiTheme }) => css`

display: inline-block;

position: relative;

bottom: ${euiTheme.size.xxl};

left: ${euiTheme.size.m};

z-index: 1;

border-radius: ${euiTheme.border.radius.medium};

${useEuiShadow('l')}

`}

>

kdelemme · 2024-11-18T19:06:28Z

Thanks @cee-chen for the review, I'll implement the suggestions

kdelemme · 2024-11-20T13:10:34Z

@cee-chen finally got time to make the changes. Thanks for the suggested changes

shahzad31 · 2024-11-20T14:00:46Z

...plugins/observability_solution/slo/public/embeddable/slo/overview/slo_embeddable_factory.tsx

+                    padding: ${euiTheme.size.xs} ${euiTheme.size.base};
+                    overflow: scroll;
+
+                    .euiAccordion__buttonContent {


might be something we can apply directly in the acordion in group list view.

I'd like to keep the goal of this PR solely on removing the deprecated theme provider and migrating to emotion.

elasticmachine · 2024-11-20T14:10:40Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 8a4add9
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-200248-8a4add95945a

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #13 / AllCases empty table should render the create new case link when the user has create privileges
[job] [logs] Jest Tests #12 / Policy Advanced Settings section should expand and collapse section when button is clicked

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`slo`	867	854	-13

Async chunks

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

id	before	after	diff
`slo`	865.7KB	842.8KB	-22.9KB

Page load bundle

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

id	before	after	diff
`slo`	28.7KB	28.6KB	-95.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`slo`	16	17	+1

Total ESLint disabled count

id	before	after	diff
`slo`	20	21	+1

History

💛 Build #251949 was flaky dd8092c
💔 Build #251556 failed aa362a4

shahzad31

Smoke testing looks good !

kibanamachine · 2024-11-20T15:37:52Z

Starting backport for target branches: 8.x

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

…onents (elastic#200248) (cherry picked from commit 684a130)

kibanamachine · 2024-11-20T15:42:40Z

💚 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

…d-components (#200248) (#200969) # Backport This will backport the following commits from `main` to `8.x`: - [chore(slo): remove deprecated theme provider and usage of styled-components (#200248)](#200248)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Kevin Delemme <[email protected]>

…onents (elastic#200248)

kdelemme added 3 commits November 14, 2024 13:54

Replace styled-components

6e8e779

Migrate from styled api to css

3287d9d

Remove the deprecated ThemeProvider

c3111fc

kdelemme added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:obs-ux-management Observability Management User Experience Team v8.17.0 labels Nov 14, 2024

kdelemme marked this pull request as ready for review November 14, 2024 19:37

kdelemme requested review from a team as code owners November 14, 2024 19:37

[CI] Auto-commit changed files from 'make api-docs'

aa362a4

botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 14, 2024

Ikuni17 approved these changes Nov 15, 2024

View reviewed changes

Merge branch 'main' into chore/remove-styled-components

dd8092c

kdelemme commented Nov 18, 2024

View reviewed changes

cee-chen reviewed Nov 18, 2024

View reviewed changes

kdelemme added 2 commits November 20, 2024 07:56

Merge branch 'main' into chore/remove-styled-components

2c309e4

Use euiTheme

8a4add9

shahzad31 reviewed Nov 20, 2024

View reviewed changes

shahzad31 approved these changes Nov 20, 2024

View reviewed changes

kdelemme merged commit 684a130 into elastic:main Nov 20, 2024
26 checks passed

kibanamachine added the v9.0.0 label Nov 20, 2024

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 20, 2024

chore(slo): remove deprecated theme provider and usage of styled-comp…

7d03123

…onents (elastic#200248) (cherry picked from commit 684a130)

kibanamachine mentioned this pull request Nov 20, 2024

[8.x] chore(slo): remove deprecated theme provider and usage of styled-components (#200248) #200969

Merged

TattdCodeMonkey pushed a commit to TattdCodeMonkey/kibana that referenced this pull request Nov 21, 2024

chore(slo): remove deprecated theme provider and usage of styled-comp…

4b19f7a

…onents (elastic#200248)

paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024

chore(slo): remove deprecated theme provider and usage of styled-comp…

46cd072

…onents (elastic#200248)

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024

chore(slo): remove deprecated theme provider and usage of styled-comp…

56e22e5

…onents (elastic#200248)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(slo): remove deprecated theme provider and usage of styled-components #200248

chore(slo): remove deprecated theme provider and usage of styled-components #200248

kdelemme commented Nov 14, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

kdelemme Nov 18, 2024

cee-chen left a comment

cee-chen Nov 18, 2024

cee-chen Nov 18, 2024

cee-chen Nov 18, 2024

cee-chen Nov 18, 2024

cee-chen Nov 18, 2024

kdelemme commented Nov 18, 2024

kdelemme commented Nov 20, 2024

shahzad31 Nov 20, 2024

kdelemme Nov 20, 2024

elasticmachine commented Nov 20, 2024 •

edited

Loading

ESLint disabled line counts

Total ESLint disabled count

shahzad31 left a comment

kibanamachine commented Nov 20, 2024

kibanamachine commented Nov 20, 2024

chore(slo): remove deprecated theme provider and usage of styled-components #200248

chore(slo): remove deprecated theme provider and usage of styled-components #200248

Conversation

kdelemme commented Nov 14, 2024 • edited by kibanamachine Loading

🖌️ Summary

🍵 Testing

elasticmachine commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

🤖 GitHub comments

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdelemme commented Nov 18, 2024

kdelemme commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 20, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

shahzad31 left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 20, 2024

kibanamachine commented Nov 20, 2024

💚 All backports created successfully

Questions ?

kdelemme commented Nov 14, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Nov 20, 2024 •

edited

Loading