Skip to content

[8.x] [Lens/SCSS] Replace scss to css-in-js for Lens codebase (#209768)#215974

Merged
nickofthyme merged 10 commits intoelastic:8.xfrom
nickofthyme:backport/8.x/pr-209768
Mar 31, 2025
Merged

[8.x] [Lens/SCSS] Replace scss to css-in-js for Lens codebase (#209768)#215974
nickofthyme merged 10 commits intoelastic:8.xfrom
nickofthyme:backport/8.x/pr-209768

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Mar 26, 2025

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

@mbondyra
[Lens/SCSS] Replace scss to css-in-js for Lens codebase (elastic#209768)
19e5356 
Replace SCSS in css-in-js for Lens codebase

(cherry picked from commit de52f41)

# Conflicts:
#	src/platform/packages/shared/kbn-chart-icons/src/assets/drop_illustration.tsx
#	x-pack/platform/plugins/shared/lens/public/datasources/form_based/datapanel.scss
#	x-pack/platform/plugins/shared/lens/public/datasources/form_based/dimension_panel/dimension_editor.scss
#	x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/formula/editor/formula.scss
#	x-pack/platform/plugins/shared/lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx
#	x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss
#	x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx
#	x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.scss
#	x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/frame_layout.scss
#	x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss
#	x-pack/platform/plugins/shared/lens/public/shared_components/dataview_picker/trigger.test.tsx
#	x-pack/platform/plugins/shared/lens/public/trigger_actions/open_lens_config/helpers.scss
#	x-pack/platform/plugins/shared/lens/public/visualizations/datatable/components/dimension_editor.test.tsx
#	x-pack/platform/plugins/shared/lens/public/visualizations/datatable/components/table_basic.scss
#	x-pack/platform/plugins/shared/lens/public/visualizations/datatable/components/table_basic.tsx
@nickofthyme nickofthyme added the backport This PR is a backport of another PR label Mar 26, 2025
@nickofthyme nickofthyme enabled auto-merge (squash) March 26, 2025 01:56
@nickofthyme nickofthyme mentioned this pull request Mar 26, 2025
Merged
37 tasks
mariairiartef
mariairiartef reviewed Mar 27, 2025
View reviewed changes
...platform/plugins/shared/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.tsx
<div
className="lnsDataPanelWrapper"
data-test-subj="lnsDataPanelWrapper"
css={css`
Copy link
Contributor

@mariairiartef mariairiartef Mar 27, 2025

Choose a reason for hiding this comment

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

Note, this property is missing: https://github.com/elastic/kibana/blob/8.x/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.scss#L4, does it make an impact?

Copy link
Contributor Author

@nickofthyme nickofthyme Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This was also removed in #204839.

Idk what the best conversion would be but also I don't see any change in UI when removing this in 8.x.

@markov00 any thoughts?

mariairiartef
mariairiartef reviewed Mar 27, 2025
View reviewed changes
x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/frame_layout.tsx
)}
css={[
sidebarStyles(euiThemeContext),
css`
Copy link
Contributor

@mariairiartef mariairiartef Mar 27, 2025

Choose a reason for hiding this comment

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

we can found this additional style https://github.com/elastic/kibana/blob/8.x/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/frame_layout.scss#L101

Copy link
Contributor Author

@nickofthyme nickofthyme Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This was also removed in #204839.

Idk what the best conversion would be but also I don't see a major change in UI when removing this in 8.x.

@markov00 any thoughts?

mariairiartef
mariairiartef reviewed Mar 28, 2025
View reviewed changes
.../lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx
display: flex;
flex-direction: column;
& > * + * {
border-top: ${euiTheme.border.thin};
Copy link
Contributor

@mariairiartef mariairiartef Mar 28, 2025

Choose a reason for hiding this comment

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

I think this style is not applied:
Screenshot 2025-03-28 at 16 21 34

Is missing the borders that the 8.x version has 🤔 :
Screenshot 2025-03-28 at 16 24 17

Copy link
Contributor Author

@nickofthyme nickofthyme Mar 28, 2025

Choose a reason for hiding this comment

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

Good catch, but this is also the case in main where these styles are not applied.

image

Would you mind opening a PR to fix this?

Copy link
Contributor

@mariairiartef mariairiartef Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sure. I opened this issue #216436

@mariairiartef

This comment was marked as duplicate.

Sign in to view
@mariairiartef

This comment was marked as duplicate.

Sign in to view
@mariairiartef mariairiartef mentioned this pull request Mar 31, 2025
Closed
mariairiartef
mariairiartef approved these changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@mariairiartef mariairiartef left a comment

Choose a reason for hiding this comment

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

I've tested it locally and I couldn't see any regressions. Thanks for taking care of this! 🤗

@nickofthyme nickofthyme merged commit ba172d1 into elastic:8.x Mar 31, 2025
8 checks passed
@nickofthyme nickofthyme deleted the backport/8.x/pr-209768 branch March 31, 2025 16:35
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 31, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 1499 1328 -171

Async chunks

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

id before after diff
eventAnnotationListing 214.1KB 214.2KB +105.0B
lens 1.5MB 1.4MB -85.1KB
total -85.0KB

Page load bundle

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

id before after diff
lens 58.7KB 58.5KB -175.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
lens 22 24 +2

Total ESLint disabled count

id before after diff
lens 22 24 +2

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariairiartef mariairiartef mariairiartef approved these changes

@kibanamachine kibanamachine Awaiting requested review from kibanamachine

Assignees

No one assigned

Labels

backport This PR is a backport of another PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nickofthyme @mariairiartef @elasticmachine @mbondyra @kibanamachine