[Platform security] Replace SCSS with CSS in JS (Part 1 - Spaces plugin)#214798
Conversation
|
Pinging @elastic/kibana-security (Team:Security) |
MichaelMarcialis
left a comment
There was a problem hiding this comment.
Thanks, @SiddharthMantri. I've left a few comments/questions below for your review. Nothing major, so assuming you're able to address them, I'll go ahead and approve now.
x-pack/platform/plugins/shared/spaces/public/space_selector/components/space_cards.tsx
Outdated
Show resolved
Hide resolved
.../platform/plugins/shared/spaces/public/management/components/section_panel/section_panel.tsx
Show resolved
Hide resolved
| css={css` | ||
| &:focus { | ||
| outline: none; | ||
| text-decoration: underline; | ||
| } | ||
| `} | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Are these styles and tabIndex necessary? If not, I'd suggest removing.
There was a problem hiding this comment.
Good question. Tab index is necessary to prevent the header from being selectable via tab. (h1 should not be a keyboard selectable element). As for the focus, i think this is just to replicate behavior from main which toggles the underline on/off focus. Happy to remove it - what do you think?
There was a problem hiding this comment.
Prior to your changes, this element was being given tabIndex={0} for some reason (not sure why). If we simply remove any tabIndex attribute and these styles, the h1 should no longer be focusable, right? If so, I say do it.
There was a problem hiding this comment.
You're right, i think removing it makes sense. The change itself was from a slack thread regarding this PR in #eui.
I misspoke! I believe we need tabIndex for this specific functionality: https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/spaces/public/space_selector/space_selector.tsx#L68
Keeping it at -1 will allow screen readers to announce (programmatic focusing) but will prevent it from keyboard focus.
x-pack/platform/plugins/shared/spaces/public/space_selector/space_selector.tsx
Outdated
Show resolved
Hide resolved
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
jeramysoucy
left a comment
There was a problem hiding this comment.
Looks good, Sid! I think the test snapshot for the manage spaces button is wrong.
Nit: I think my only ask would be for more consistency in how the css is applied in each component - sometimes there is a styles object containing all the needed styles, other times it is passed in as a property, and sometimes it is just implemented inline. It would be nice it this was consistent throughout, but it is not necessarily an issue as-is.
| const { objects } = summarizedCopyResult; | ||
| const { euiTheme } = useEuiTheme(); | ||
|
|
||
| const styles = { |
There was a problem hiding this comment.
Observation/question: Do you think it would be worth moving all of our styles to a common location and referencing them, or keeping them inline?
If we keep them inline, would it be more consistent to keep each style with the component where it is used, rather than passing styles as properties (like with ResolveAllConflicts)?
| const styles = { | ||
| collapsiblePanelLogo: css({ | ||
| verticalAlign: 'text-bottom', | ||
| marginRight: euiTheme.size.s, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
Q: Is there a reason to use this nested styles object? Is it a convention we are adopting for making sure all styles in a component get defined in one place?
.../platform/plugins/shared/spaces/public/management/components/section_panel/section_panel.tsx
Show resolved
Hide resolved
|
|
||
| exports[`ManageSpacesButton doesn't render if user profile forbids managing spaces 1`] = `""`; | ||
| exports[`ManageSpacesButton doesn't render if user profile forbids managing spaces 1`] = ` | ||
| <ManageSpacesButtonUI |
There was a problem hiding this comment.
This seems pretty wild. Is this expected?
There was a problem hiding this comment.
I believe it's by design. The new component wraps the EUI Theme using the withEuiTheme HOC which inserts all the properties to the component. It's the equivalent of useEuiTheme but for class components. One way to get around the bloated snap could be to convert ManageSpacesButton to a functional component but i couldn't decide if it makes sense to do as part of this PR. What do you think?
There was a problem hiding this comment.
If it doesn't cause significant delay to this PR, then I say why not. Otherwise, we can just open an issue to convert it later.
Completely agreed. Fix incoming! |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
| public render() { | ||
| if (!this.props.capabilities.spaces.manage) { | ||
| return null; | ||
| export const ManageSpacesButton: React.FC<Props> = ({ |
…in) (elastic#214798) ## Summary Part of elastic#211652 Removed most SASS files from the Spaces plugin. (Full checklist on parent issue) Remaining file: `x-pack/platform/plugins/shared/spaces/public/space_selector/space_selector.scss` This file requires a custom mixin that we'll need to migrate once this PR: https://github.com/elastic/kibana/pull/214729/files lands. It introduces a `cssUtils` file to ensure consistency in Kibana specific mixins. ### 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: - Copy Saved Objects to Space flyout - Share Saved Objects to Space flyout - Space selector screen - Space editing screen - Space selector drop down menu in Nav Bar Ideally, you should see no visual regression between the two versions. ## Screenshots | Component | Main | PR | |--------|--------|--------| | Space Edit | <img width="300" alt="space_edit_main" src="https://github.com/user-attachments/assets/786feeb7-5047-443c-bb63-41e90e31a82b" /> | <img width="300" alt="space_edit_pr" src="https://github.com/user-attachments/assets/975cc096-25d7-4bd5-804d-f82f65a908bf" /> | | Space selector nav bar | <img width="300" alt="space_selector_nav_bar_main" src="https://github.com/user-attachments/assets/c6c05d28-3dfa-43c2-9586-b66a24f990d6" /> | <img width="317" alt="Screenshot 2025-03-20 at 09 11 50" src="https://github.com/user-attachments/assets/277d3094-640b-4604-adc7-5c8465aeb21c" /> | | Share to space | <img width="300" alt="share_to_space_main" src="https://github.com/user-attachments/assets/5782a314-66f7-4780-bcfb-b0a85cece035" /> | <img width="300" alt="share_to_space_pr" src="https://github.com/user-attachments/assets/73a48305-7fa7-4637-9856-60461cbad770" /> | | Copy to Space flyout | <img width="300" alt="copy_to_space_pr" src="https://github.com/user-attachments/assets/54342ca2-b2e1-4844-a66f-fae512ff8910" /> | <img width="300" alt="copy_to_space_main" src="https://github.com/user-attachments/assets/a629f12a-75c4-4ba6-a7cf-cdeca1310ef3" /> | | Copy to Space confirmation | <img width="300" alt="copy_to_space_confirmed_main" src="https://github.com/user-attachments/assets/78f93d73-e789-487f-94c1-eebcef7ce183" /> | <img width="300" alt="copy_to_space_confirmed_pr" src="https://github.com/user-attachments/assets/2020e71a-88b4-4107-9b05-ae90bf7d39f1" /> | | Space selector | <img width="300" alt="Space_selector_before" src="https://github.com/user-attachments/assets/b8ed7269-e6f6-4bc0-bb24-1c53ac451083" /> | <img width="300" alt="Space_selector_after" src="https://github.com/user-attachments/assets/770d2141-8642-483f-b72c-bce6d5ebd282" /> | ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…in) (elastic#214798) ## Summary Part of elastic#211652 Removed most SASS files from the Spaces plugin. (Full checklist on parent issue) Remaining file: `x-pack/platform/plugins/shared/spaces/public/space_selector/space_selector.scss` This file requires a custom mixin that we'll need to migrate once this PR: https://github.com/elastic/kibana/pull/214729/files lands. It introduces a `cssUtils` file to ensure consistency in Kibana specific mixins. ### 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: - Copy Saved Objects to Space flyout - Share Saved Objects to Space flyout - Space selector screen - Space editing screen - Space selector drop down menu in Nav Bar Ideally, you should see no visual regression between the two versions. ## Screenshots | Component | Main | PR | |--------|--------|--------| | Space Edit | <img width="300" alt="space_edit_main" src="https://github.com/user-attachments/assets/786feeb7-5047-443c-bb63-41e90e31a82b" /> | <img width="300" alt="space_edit_pr" src="https://github.com/user-attachments/assets/975cc096-25d7-4bd5-804d-f82f65a908bf" /> | | Space selector nav bar | <img width="300" alt="space_selector_nav_bar_main" src="https://github.com/user-attachments/assets/c6c05d28-3dfa-43c2-9586-b66a24f990d6" /> | <img width="317" alt="Screenshot 2025-03-20 at 09 11 50" src="https://github.com/user-attachments/assets/277d3094-640b-4604-adc7-5c8465aeb21c" /> | | Share to space | <img width="300" alt="share_to_space_main" src="https://github.com/user-attachments/assets/5782a314-66f7-4780-bcfb-b0a85cece035" /> | <img width="300" alt="share_to_space_pr" src="https://github.com/user-attachments/assets/73a48305-7fa7-4637-9856-60461cbad770" /> | | Copy to Space flyout | <img width="300" alt="copy_to_space_pr" src="https://github.com/user-attachments/assets/54342ca2-b2e1-4844-a66f-fae512ff8910" /> | <img width="300" alt="copy_to_space_main" src="https://github.com/user-attachments/assets/a629f12a-75c4-4ba6-a7cf-cdeca1310ef3" /> | | Copy to Space confirmation | <img width="300" alt="copy_to_space_confirmed_main" src="https://github.com/user-attachments/assets/78f93d73-e789-487f-94c1-eebcef7ce183" /> | <img width="300" alt="copy_to_space_confirmed_pr" src="https://github.com/user-attachments/assets/2020e71a-88b4-4107-9b05-ae90bf7d39f1" /> | | Space selector | <img width="300" alt="Space_selector_before" src="https://github.com/user-attachments/assets/b8ed7269-e6f6-4bc0-bb24-1c53ac451083" /> | <img width="300" alt="Space_selector_after" src="https://github.com/user-attachments/assets/770d2141-8642-483f-b72c-bce6d5ebd282" /> | ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>
) Closes elastic#211652 ## Summary Part 2 of elastic#211652 and follow up from elastic#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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co> (cherry picked from commit 589c0b4) # Conflicts: # src/platform/plugins/private/interactive_setup/public/app.scss # src/platform/plugins/private/interactive_setup/public/app.tsx # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.scss # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.test.tsx # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx # x-pack/platform/plugins/shared/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap # x-pack/platform/plugins/shared/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap # x-pack/platform/plugins/shared/security/public/authentication/login/login_page.scss # x-pack/platform/plugins/shared/security/public/authentication/login/login_page.tsx # x-pack/platform/plugins/shared/security/public/authentication/overwritten_session/__snapshots__/overwritten_session_page.test.tsx.snap # x-pack/platform/plugins/shared/security/tsconfig.json
) Closes elastic#211652 ## Summary Part 2 of elastic#211652 and follow up from elastic#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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co> (cherry picked from commit 589c0b4) # Conflicts: # src/platform/plugins/private/interactive_setup/public/app.scss # src/platform/plugins/private/interactive_setup/public/app.tsx # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.scss # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.test.tsx # x-pack/platform/plugins/shared/security/public/authentication/components/authentication_state_page/authentication_state_page.tsx # x-pack/platform/plugins/shared/security/public/authentication/login/__snapshots__/login_page.test.tsx.snap # x-pack/platform/plugins/shared/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snap # x-pack/platform/plugins/shared/security/public/authentication/login/components/login_form/login_form.scss # x-pack/platform/plugins/shared/security/public/authentication/login/login_page.scss # x-pack/platform/plugins/shared/security/public/authentication/login/login_page.tsx # x-pack/platform/plugins/shared/security/public/authentication/overwritten_session/__snapshots__/overwritten_session_page.test.tsx.snap # x-pack/platform/plugins/shared/security/tsconfig.json
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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>
) Closes elastic#211652 ## Summary Part 2 of elastic#211652 and follow up from elastic#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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>
) Closes elastic#211652 ## Summary Part 2 of elastic#211652 and follow up from elastic#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|<img width="300" alt="Screenshot 2025-05-09 at 10 31 18" src="https://github.com/user-attachments/assets/483d9e47-e4a0-4e01-8282-9d205ba0bf6d" />|<img width="300" alt="Screenshot 2025-05-09 at 10 54 14" src="https://github.com/user-attachments/assets/62965843-5242-4d96-b300-812cdc349864" />| |Login page|<img width="300" alt="Screenshot 2025-05-08 at 17 05 23" src="https://github.com/user-attachments/assets/99536f62-75d8-46e2-ae3a-aba7b990a723" />|<img width="300" alt="Screenshot 2025-05-08 at 18 24 50" src="https://github.com/user-attachments/assets/3c2aadc9-1746-4d7f-9093-a6053916b597" />| |Role mapping rule editor| <img width="300" src="https://github.com/user-attachments/assets/8a14e323-223d-43df-9dfb-edf8f6859c40" />|<img width="300" src="https://github.com/user-attachments/assets/68a9b37d-060d-4060-b2ad-b0b3319de78d" />| ### 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) ### 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. - [x] The risk of inexact conversion: verifying this PR requires manual checks to ensure that the conversion has not created any regressions in the style. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kurt <kc13greiner@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>
Summary
Part of #211652
Removed most SASS files from the Spaces plugin. (Full checklist on parent issue)
Remaining file:
x-pack/platform/plugins/shared/spaces/public/space_selector/space_selector.scssThis file requires a custom mixin that we'll need to migrate once this PR: https://github.com/elastic/kibana/pull/214729/files lands. It introduces a
cssUtilsfile to ensure consistency in Kibana specific mixins.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:
Once started, in a private browsing window, you should have access to Kibana on main on
localhost:5602On 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:
Ideally, you should see no visual regression between the two versions.
Screenshots
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*label is applied per the guidelinesIdentify 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.