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 Getting Started page follows Dashboard themes #1538

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

AMoo-Miki
Copy link
Contributor

@AMoo-Miki AMoo-Miki commented Aug 2, 2023

Description

  1. public/assets/get_started.svg was an atrocious 18KB asset; cleaned it up to 3.5KB.
  2. Created 2 layers out of get_started.svg.
  3. OSD doesn't provide svgr or anything for inlining SVGs so can't use currentColor; I have put it in the source SVGs anyway in case it is added in the future.
  4. This uses CSS masking.

security-get-started

Issues Resolved

#1527 - Part 1

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.

@peternied
Copy link
Member

Based on this comment, Please don't merge until I can validate it across relevant browsers. I have marked this pull request a draft, please let us know if we can help, thanks!

@AMoo-Miki
Copy link
Contributor Author

@peternied are these tests supposed to be failing?

@peternied
Copy link
Member

peternied commented Aug 2, 2023

@RyanL1997 I believe you were looking into these failures, could you link the issue tracking it?

@AMoo-Miki AMoo-Miki force-pushed the themed-image branch 2 times, most recently from e0569bb to 7bfd411 Compare August 3, 2023 01:01
@AMoo-Miki AMoo-Miki marked this pull request as ready for review August 3, 2023 01:02
Signed-off-by: Miki <miki@amazon.com>
@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #1538 (5570340) into main (7d34953) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1538   +/-   ##
=======================================
  Coverage   66.06%   66.06%           
=======================================
  Files          93       93           
  Lines        2328     2328           
  Branches      310      310           
=======================================
  Hits         1538     1538           
  Misses        722      722           
  Partials       68       68           
Files Changed Coverage Δ
public/apps/configuration/panels/get-started.tsx 76.92% <100.00%> (ø)

@cwperks
Copy link
Member

cwperks commented Aug 3, 2023

@AMoo-Miki The failing checks now are related to compilation issues in the backend security plugin repo. There's an open PR here: opensearch-project/security#3091

There's potentially another issue with some selenium tests and the latest version of FF: #1533 (comment)

Looking into the FF issue now. I will re-run the failed checks when the backend PR is merged and also reply back if any rebasing is needed.

Thank you for the PR. This change looks good to me.

@stephen-crawford
Copy link
Contributor

Thank you @AMoo-Miki. This is great! I appreciate all your help with resolving this.

@cwperks
Copy link
Member

cwperks commented Aug 3, 2023

@AMoo-Miki This can be rebased now. The test failures you are seeing were due to an issue with the latest version of the selenium-webdriver: #1540.

@cwperks cwperks added the backport 2.x backport to 2.x branch label Aug 4, 2023
Copy link
Collaborator

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Thanks @AMoo-Miki. I'm re-running the unit tests now.

@AMoo-Miki
Copy link
Contributor Author

Is there anything more I can do for you on this PR?

@cwperks
Copy link
Member

cwperks commented Aug 10, 2023

Nothing from me. This looks good to merge.

@davidlago davidlago merged commit 20ba268 into opensearch-project:main Aug 10, 2023
22 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 10, 2023
Signed-off-by: Miki <miki@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
(cherry picked from commit 20ba268)
samuelcostae pushed a commit to samuelcostae/security-dashboards-plugin that referenced this pull request Aug 16, 2023
Signed-off-by: Miki <miki@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
Signed-off-by: Sam <samuel.costa@eliatra.com>
RyanL1997 pushed a commit that referenced this pull request Aug 16, 2023
* Make the steps diagram honor the theme (#1538)

Signed-off-by: Miki <miki@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
(cherry picked from commit 20ba268)

* Update jwt_auth.test.ts

Extend timeout for 2.x line

Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>

---------

Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
DarshitChanpura pushed a commit that referenced this pull request Aug 18, 2023
* Make the steps diagram honor the theme (#1538)

Signed-off-by: Miki <miki@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
Signed-off-by: Sam <samuel.costa@eliatra.com>

* Adding testing info do DEVELOPER_GUIDE.md

Signed-off-by: Sam <samuel.costa@eliatra.com>

* Minor changes and adding link

Signed-off-by: Sam <samuel.costa@eliatra.com>

---------

Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Sam <samuel.costa@eliatra.com>
Co-authored-by: Miki <miki@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
@peternied peternied changed the title Make the steps diagram honor the theme Security getting started follows Dashboard themes Aug 31, 2023
@peternied peternied changed the title Security getting started follows Dashboard themes Security getting started page follows Dashboard themes Aug 31, 2023
@peternied peternied changed the title Security getting started page follows Dashboard themes Security Getting Started page follows 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.

6 participants