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

Security Roles and Audit Log Settings follow Dashboard themes #1558

Merged

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Aug 18, 2023

Description

There are 2 instances in this repo of hardcoded colors:

public/apps/configuration/_index.scss
 17:3  ✖  Using the value "#687078" is not allowed. All colors should be inherited from the OUI theme, not hard-coded. Either remove the custom color rule or use OUI SASS variables instead. (no_restricted_values)  @osd/stylelint/no_restricted_values

public/apps/configuration/panels/audit-logging/_index.scss
 10:3  ✖  Using the value "#666" is not allowed. All colors should be inherited from the OUI theme, not hard-coded. Either remove the custom color rule or use OUI SASS variables instead. (no_restricted_values)  @osd/stylelint/no_restricted_values

This PR either updates or removes the instance of the hardcoded color to comply with #1471

Category

Maintenance

Issues Resolved

#1471

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@@ -5,7 +5,3 @@
.form-row {
max-width: 800px;
}

pre code {
Copy link
Member Author

@cwperks cwperks Aug 18, 2023

Choose a reason for hiding this comment

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

Before:

Screenshot 2023-08-18 at 1 45 40 PM

After:

Screenshot 2023-08-18 at 1 45 52 PM

This is on Security -> Audit Logs -> Compliance Settings (Configure)

This could use a more subdued color, but I thought it would be reasonable to remove and use the colors shipped with EuiCodeEditor.

@@ -14,7 +14,7 @@
*/

.panel-header-count {
color: #687078;
color: $euiTextSubduedColor;
Copy link
Member Author

@cwperks cwperks Aug 18, 2023

Choose a reason for hiding this comment

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

Before:

Screenshot 2023-08-18 at 1 47 52 PM

After:

Screenshot 2023-08-18 at 1 52 03 PM

This affects the number in paren next to the config type. In this case Roles **(42)**. This class is re-used on a few panels.

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #1558 (8899371) into main (20ba268) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1558   +/-   ##
=======================================
  Coverage   66.06%   66.06%           
=======================================
  Files          93       93           
  Lines        2328     2328           
  Branches      310      310           
=======================================
  Hits         1538     1538           
  Misses        722      722           
  Partials       68       68           

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

Nice!

@cwperks
Copy link
Member Author

cwperks commented Aug 21, 2023

@kamingleung Could you review this small security-dashboards-plugin PR to remove 2 instances of hardcoded colors?

@stephen-crawford stephen-crawford merged commit c10031f into opensearch-project:main Aug 22, 2023
13 checks passed
@cwperks cwperks added the backport 2.x backport to 2.x branch label Aug 22, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 22, 2023
Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit c10031f)
cwperks added a commit that referenced this pull request Aug 22, 2023
Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit c10031f)

Co-authored-by: Craig Perkins <[email protected]>
@peternied peternied changed the title Update or remove instances of hardcoded colors Security Roles and Audit Log Settings follow Dashboard themes Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants