Skip to content

[Platform security] Replace SCSS with CSS in JS (part 2)#220508

Closed
SiddharthMantri wants to merge 26 commits intoelastic:mainfrom
SiddharthMantri:security-scss-cssjs
Closed

[Platform security] Replace SCSS with CSS in JS (part 2)#220508
SiddharthMantri wants to merge 26 commits intoelastic:mainfrom
SiddharthMantri:security-scss-cssjs

Conversation

@SiddharthMantri
Copy link
Contributor

@SiddharthMantri SiddharthMantri commented May 8, 2025

Closes #211652

Summary

Part 2 of #211652 and follow up from #214798

Removed most SASS files from all platform security owned code. (Full checklist on parent issue)

How to test

Testing visual regression isn't super straightforward here. For my local testing, i started two instances of Kibana (main and this branch)

On main:

  • Start es:
yarn es snapshot --license=trial -E http.port=9400
  • Start kibana with the following config (CLI or kibana.dev.yml)
server.port: 5602
elasticsearch.hosts: ["http://localhost:9400"]

Once started, in a private browsing window, you should have access to Kibana on main on localhost:5602

On this PR:
Start ES and Kibana normally (Kibana should be available on localhost:5601)

This PR contains changes to the following parts of the Spaces plugin:

  • Access agreement page
  • Login page (see note below regarding testing)
  • Role mapping rule group editor

For testing the full login page with different selectors, etc, you can start kibana with the following changes in the kibana.dev.yml file

xpack.security.authc.providers:
  basic.basic1:
    order: 0
    hint: "Use your corporate username & password"
    accessAgreement:
      message: |
        **You are accessing a system with sensitive information**

        By logging in, you acknowledge that information system usage

  pki.pki1:
    order: 1

  token.token1:
    order: 2

  saml.saml1:
    order: 3
    realm: saml1

  oidc.oidc1:
    order: 4
    realm: oidc1

Ideally, you should see no visual regression between the two versions.

Screenshots

Component Main PR
Access agreement page Screenshot 2025-05-09 at 10 31 18 Screenshot 2025-05-09 at 10 54 14
Login page Screenshot 2025-05-08 at 17 05 23 Screenshot 2025-05-08 at 18 24 50
Role mapping rule editor

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

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

  • The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style.

@SiddharthMantri SiddharthMantri added Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// technical debt Improvement of the software architecture and operational architecture release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.1.0 scss-removal labels May 9, 2025
@SiddharthMantri
Copy link
Contributor Author

@elasticmachine merge upstream

@SiddharthMantri SiddharthMantri marked this pull request as ready for review May 9, 2025 09:04
@SiddharthMantri SiddharthMantri requested review from a team as code owners May 9, 2025 09:04
@elasticmachine
Copy link
Contributor

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

@kc13greiner kc13greiner self-requested a review May 19, 2025 18:36
return (
<div className="interactiveSetup">
<header className="interactiveSetup__header eui-textCenter">
<div css={fullScreenGraphicsMixinStyles(Number(euiTheme.levels.navigation), theme)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a slight regression somewhere in these Interactive Setup changes based on my testing:

Main:
Screenshot 2025-05-19 at 3 25 00 PM

This PR:
Screenshot 2025-05-19 at 3 41 25 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kc13greiner - That's right! I need to get this fixed and get the CI to pass

@SiddharthMantri
Copy link
Contributor Author

@elasticmachine merge upstream

@SiddharthMantri
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

elasticmachine commented Jun 3, 2025

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #108 / APM API tests settings/anomaly_detection/update_to_v3.spec.ts trial no archive Updating ML jobs to v3 when there are only v2 jobs "after all" hook for "creates a new job for each environment that has a v2 job"
  • [job] [logs] FTR Configs #108 / APM API tests settings/anomaly_detection/update_to_v3.spec.ts trial no archive Updating ML jobs to v3 when there are only v2 jobs "after all" hook for "creates a new job for each environment that has a v2 job"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
interactiveSetup 42 24 -18
security 591 541 -50
spaces 236 226 -10
total -78

Async chunks

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

id before after diff
security 516.2KB 476.1KB -40.2KB
spaces 210.1KB 207.7KB -2.3KB
total -42.5KB

Page load bundle

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

id before after diff
interactiveSetup 38.6KB 31.8KB -6.8KB
security 60.3KB 60.5KB +148.0B
total -6.7KB
Unknown metric groups

async chunk count

id before after diff
security 20 21 +1

miscellaneous assets size

id before after diff
interactiveSetup 1.1MB 0.0B -1.1MB
security 1.3MB 161.8KB -1.1MB
total -2.3MB

History

@SiddharthMantri SiddharthMantri marked this pull request as draft August 29, 2025 14:43
@elasticmachine
Copy link
Contributor

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!
  • Click to trigger kibana-deploy-project-from-pr for this PR!
  • Click to trigger kibana-deploy-cloud-from-pr for this PR!

@SiddharthMantri
Copy link
Contributor Author

Closing in favor of #233508

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

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes scss-removal Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// technical debt Improvement of the software architecture and operational architecture v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Platform Security] Replace SCSS with CSS-in-JS

5 participants