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

Create OuiSimplifiedBreadcrumbs and update OuiHeaderBreadcrumbs #1374

Merged

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented Aug 26, 2024

Description

Create OuiSimplifiedBreadcrumbs and update OuiHeaderBreadcrumbs
image

Issues Resolved

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Update CHANGELOG.md
  • 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.

const classes = classNames(
'ouiHeaderBreadcrumbs',
{
'ouiHeaderBreadcrumbs--simplified': simplify,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this needed here or should this just be in the component itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it's needed here, to make use of the existing class generator. ouiHeaderBreadcrumbs--simplified is added to apply the spacing for the new breadcrumbs

margin-right: $ouiBreadcrumbSpacing;
width: 2px;
height: $ouiSize;
transform: translateY(-1px) rotate(15deg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a pre-existing implementation of the slashes?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, taken from the code in oui 1.1

@virajsanghvi
Copy link
Collaborator

Can you add a changelog entry?

Signed-off-by: Zhongnan Su <szhongna@amazon.com>
@zhongnansu
Copy link
Member Author

Can you add a changelog entry?

added

@virajsanghvi virajsanghvi merged commit 2c0c73c into opensearch-project:main Aug 26, 2024
13 of 14 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 26, 2024
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
(cherry picked from commit 2c0c73c)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
virajsanghvi pushed a commit that referenced this pull request Aug 27, 2024
… (#1375)

(cherry picked from commit 2c0c73c)

Signed-off-by: Zhongnan Su <szhongna@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

2 participants