-
Notifications
You must be signed in to change notification settings - Fork 9
Admin: Add new Breadcrumbs component
#4936
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
base: feature/admin-breadcrumbs
Are you sure you want to change the base?
Conversation
95b432e to
dcbef71
Compare
dcbef71 to
a61622d
Compare
a61622d to
8ce54ee
Compare
Breadcrumbs component
Co-authored-by: Ricky James Smith <[email protected]>
Co-authored-by: Ricky James Smith <[email protected]>
|
|
||
| type BreadcrumbsClassKey = "root" | "breadcrumbsItem" | "separator"; | ||
|
|
||
| export interface BreadcrumbsItem { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming: instead of BreadcrumbsItem we could also go with Breadcrumb, but I'm not sure what's better.
| interface BreadcrumbsProps | ||
| extends ThemedComponentBaseProps<{ | ||
| root: "div"; | ||
| breadcrumbsItem: typeof Typography; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming: why add the "breadcrumbs" prefix here, but not for the other slots?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to item instead
Description
A new
Breadcrumbcomponent is added, that should be used for rendering breadcrumbs in the admin (instead of StackBreadcrumbs and ToolbarBreadcrumbs).In this PR the component is added and styled for desktop only.
Acceptance criteria
Screenshots/screencasts
Open TODOs/questions
Further information