Refactor/remove breadcrumb styling #4341
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that the breacrumb styling is coming from OUI Signed-off-by: Josh Romero <rmerqg@amazon.com>
Essentially reverting: - opensearch-project#1954 - opensearch-project#2085 Signed-off-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Josh Romero <rmerqg@amazon.com>
joshuarrrr
requested review from
ananzh,
kavilla,
seanneumann,
AMoo-Miki,
ashwin-pc,
abbyhu2000,
zengyan-amazon,
kristenTian,
zhongnansu,
manasvinibs,
ZilongX and
Flyingliuhub
as code owners
manasvinibs
reviewed
Jun 21, 2023
| @@ -114,7 +114,7 @@ export function Header({ | |||
| const toggleCollapsibleNavRef = createRef<HTMLButtonElement & { euiAnimate: () => void }>(); | |||
| const navId = htmlIdGenerator()(); | |||
| const className = classnames('hide-for-sharing', 'headerGlobalNav'); | |||
| const { useExpandedHeader = true, darkMode } = branding; | |||
| const { useExpandedHeader = true } = branding; | |||
There was a problem hiding this comment.
Does it mean now breadcrumbs cannot be customized theme specific and is inherited as it is from oui?
There was a problem hiding this comment.
Yeah, breadcrumbs in OpenSearch Dashboards were never intended to be customized/customizable, we just had to do things that way because OUI was not yet available. But OUI should be responsible for the styling and theming of the breadcrumb components.
Codecov Report
@@ Coverage Diff @@
## feature/oui-re-theme #4341 +/- ##
=====================================================
Coverage 66.41% 66.41%
=====================================================
Files 3246 3246
Lines 62466 62465 -1
Branches 9674 9673 -1
=====================================================
Hits 41485 41485
- Misses 18606 18633 +27
+ Partials 2375 2347 -28
Flags with carried forward coverage won't be shown. Click here to find out more.
|
manasvinibs
approved these changes
Jun 29, 2023
This was referenced Jun 30, 2023
joshuarrrr
merged commit 1de54d7
into
opensearch-project:feature/oui-re-theme
43 of 44 checks passed
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-main main
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-main
# Create a new branch
git switch --create backport/backport-4341-to-main
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1de54d72f229bee57f7dcb82d38ad16cf1f2987f
# Push it to GitHub
git push --set-upstream origin backport/backport-4341-to-main
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-mainThen, create a pull request where the |
|
backport didn't work - I'll just manually cherry-pick |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This removes much of the breadcrumb customization required prior to the establishment of OUI, now that breadcrumb styles have been updated over there.
Issues Resolved
Screenshot
Testing the changes
Check List
yarn test:jestyarn test:jest_integrationyarn test:ftr