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

#3105850: Implement Breadcrumbs #23

Merged
merged 2 commits into from
Feb 14, 2022
Merged

#3105850: Implement Breadcrumbs #23

merged 2 commits into from
Feb 14, 2022

Conversation

veej
Copy link
Member

@veej veej commented Feb 9, 2022

Closes #3105850

Test Plan

tests performed

See storybook / Chromatic (Link + Breadcrumb components)

@kanbanbot kanbanbot added the WIP label Feb 9, 2022
@veej veej requested a review from gabro February 10, 2022 10:00
@kanbanbot kanbanbot added in review and removed WIP labels Feb 10, 2022
labelDecoration: BentoSprinkles["textDecoration"];
};

export function createLink(
Copy link
Member

Choose a reason for hiding this comment

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

Why Link and not ButtonLink?

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +77 to +79
<Label size="large" {...itemProps} color={undefined}>
{label}
</Label>
Copy link
Member

@gabro gabro Feb 11, 2022

Choose a reason for hiding this comment

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

Let's also backport this fix I've put up today

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I'm not sure I understand that fix. It doesn't look right to me:
image

Copy link
Member Author

@veej veej Feb 11, 2022

Choose a reason for hiding this comment

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

oh, understood now. It makes sense only if we use ButtonLink

@kanbanbot kanbanbot added WIP and removed in review labels Feb 11, 2022
@veej veej force-pushed the 3105850-implement_breadcrumbs branch from 2f9aaa8 to e69e4f9 Compare February 11, 2022 15:50
@veej veej force-pushed the 3105850-implement_breadcrumbs branch from e69e4f9 to 607bc0e Compare February 14, 2022 16:26
@veej veej requested a review from gabro February 14, 2022 16:26
@veej veej unassigned Phyele Feb 14, 2022
@veej veej requested a review from Phyele February 14, 2022 16:39
@kanbanbot kanbanbot added in review and removed WIP labels Feb 14, 2022
@veej veej merged commit c1a4682 into main Feb 14, 2022
@veej veej deleted the 3105850-implement_breadcrumbs branch February 14, 2022 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants