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

chore(oui): Bump OUI dependency to 1.2.0, remove breadcrumb styling #4170

Closed

Conversation

joshuarrrr
Copy link
Member

@joshuarrrr joshuarrrr commented May 30, 2023

Description

Bump OUI version - don't merge until we update from release candidate.

OUI 1.2 includes all necessary breadcrumb styling, so OpenSearch Dashboards customizations are no longer necessary. So we're also essentially reverting the changes from:

Issues Resolved

Screenshot

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Josh Romero <rmerqg@amazon.com>
@joshuarrrr
Copy link
Member Author

Converting to draft until v1.2.0 is released. But we can run the CI against the release candidate.

Signed-off-by: Josh Romero <rmerqg@amazon.com>
@joshuarrrr
Copy link
Member Author

I opened a separate PR for the Link Checker failure: #4171

Signed-off-by: Josh Romero <rmerqg@amazon.com>
@joshuarrrr joshuarrrr marked this pull request as ready for review May 30, 2023 18:31
@BSFishy
Copy link
Contributor

BSFishy commented May 30, 2023

Breadcrumbs probably look wonky with current changes. New breadcrumb styling went into 1.2, so the styling is most likely being applied twice. I outlined how to fix in here: opensearch-project/oui#796 (comment)

@joshuarrrr
Copy link
Member Author

Thanks @BSFishy for the instructions - I'll update the PR accordingly.

@joshuarrrr joshuarrrr marked this pull request as draft May 30, 2023 18:42
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Now that the breacrumb styling is updated in OUI v1.2.0

Signed-off-by: Josh Romero <rmerqg@amazon.com>
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #4170 (df02b28) into main (2b725a9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4170   +/-   ##
=======================================
  Coverage   66.15%   66.15%           
=======================================
  Files        3314     3314           
  Lines       63911    63910    -1     
  Branches     9962     9961    -1     
=======================================
+ Hits        42280    42282    +2     
+ Misses      19186    19185    -1     
+ Partials     2445     2443    -2     
Flag Coverage Δ
Linux_1 34.73% <0.00%> (+<0.01%) ⬆️
Linux_2 55.04% <100.00%> (+<0.01%) ⬆️
Linux_3 43.18% <0.00%> (+<0.01%) ⬆️
Linux_4 35.10% <0.00%> (+<0.01%) ⬆️
Windows_1 34.75% <0.00%> (+<0.01%) ⬆️
Windows_2 55.00% <100.00%> (+<0.01%) ⬆️
Windows_3 43.18% <0.00%> (+<0.01%) ⬆️
Windows_4 35.10% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/core/public/chrome/ui/header/header.tsx 75.00% <100.00%> (ø)
...ore/public/chrome/ui/header/header_breadcrumbs.tsx 100.00% <100.00%> (+10.00%) ⬆️

... and 1 file with indirect coverage changes

Essentially reverting:
- opensearch-project#1954
- opensearch-project#2085

Signed-off-by: Josh Romero <rmerqg@amazon.com>
@joshuarrrr joshuarrrr changed the title chore(oui): Bump OUI dependency to 1.2.0 chore(oui): Bump OUI dependency to 1.2.0, remove breadcrumb styling May 30, 2023
@joshuarrrr joshuarrrr marked this pull request as ready for review May 30, 2023 22:37
Signed-off-by: Josh Romero <rmerqg@amazon.com>
ananzh
ananzh previously approved these changes May 31, 2023
BSFishy
BSFishy previously approved these changes May 31, 2023
@kavilla
Copy link
Member

kavilla commented May 31, 2023

Discussed with @KrooshalUX:

Since anywhere project got bumped from 2.8 release. This PR is not required to get in and bake longer.

@kavilla
Copy link
Member

kavilla commented May 31, 2023

I noticed the breadcrumb got a little bit blurrer:

From playground: with expanded header turned off:
Screenshot 2023-05-31 at 11 27 17 AM

After, my local with expanded header default value:
Screenshot 2023-05-31 at 11 27 30 AM

@joshuarrrr
Copy link
Member Author

Converting to draft - we'll split the OUI changes into different releases so we can pull a clean v1.1.1 with just the icon changes.

@joshuarrrr joshuarrrr dismissed stale reviews from BSFishy and ananzh via df02b28 July 17, 2023 22:44
@joshuarrrr joshuarrrr marked this pull request as ready for review July 17, 2023 22:44
@joshuarrrr
Copy link
Member Author

This is ready for re-review. Discussed with @AMoo-Miki and we're ok to take some of the breadcrumb concerns in #4457 as a blocker for this in the 2.x line, but we'll go ahead and merge to main so we can further test and validate prior to the 2.10 release.

@abbyhu2000
Copy link
Member

@joshuarrrr Do you want to solve the conflicts? and also there are some test fails

@joshuarrrr
Copy link
Member Author

Closing in favor of #4621 - no need to bump to OUI 1.2, since we've already bumped main to OUI 1.3-rc.

@joshuarrrr joshuarrrr closed this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants