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

feat(Breadcrumb): new component #506

Merged
merged 37 commits into from
Nov 16, 2023

Conversation

Haythamasalama
Copy link
Contributor

@Haythamasalama Haythamasalama commented Aug 6, 2023

Enhance user experience with a breadcrumb that displays the website's hierarchical structure.

Resolves #157

@nuxt-studio
Copy link

nuxt-studio bot commented Aug 6, 2023

Live Preview ready!

Name Edit Preview Latest Commit
ui Edit on Studio ↗︎ View Live Preview eec51c7

@vercel
Copy link

vercel bot commented Aug 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview Nov 16, 2023 5:11pm

@Haythamasalama
Copy link
Contributor Author

Haythamasalama commented Aug 11, 2023

@benjamincanac It's ready to review .

I have a question about why the default button ULink can't be changed, for example, to something else like a div? In my case, if the to or href is not passed for Breadcrumb, it will default to being a button. I believe this might not be considered valid for semantic HTML?

Copy link
Member

Maybe we could add an as prop to choose the fallback? Like we do for the Card component: https://github.com/nuxtlabs/ui/blob/dev/src/runtime/components/layout/Card.vue#L35

Not sure about this though.

@Haythamasalama
Copy link
Contributor Author

Maybe we could add an as prop to choose the fallback? Like we do for the Card component: https://github.com/nuxtlabs/ui/blob/dev/src/runtime/components/layout/Card.vue#L35

Not sure about this though.

I think it's good, similar to headless UI. But for breadcrumb if we have tag = as and item-tag = item-as,

Copy link
Member

I was talking about the Link component.

@Haythamasalama
Copy link
Contributor Author

Haythamasalama commented Aug 14, 2023

I was talking about the Link component.

Oh, Sure, I'll create a new PR to add the as prop for ULink and thoroughly test the components that utilize it with same default button similer to Card.

@Haythamasalama
Copy link
Contributor Author

#535

@Haythamasalama
Copy link
Contributor Author

Can we please review this, @benjamincanac? This is the third time I've attempted to resolve conflicts and update it with the latest changes that have occurred.

Copy link
Member

I'm so sorry about the delay but issues and PRs keep coming in. I prioritize the work on bug fixes and optimizations such as #692, #648 or #509 for example. @nuxt/ui-pro is also getting closer to release. It takes a lot of time to review PRs about new components so I'll treat them once things settle down.

@Haythamasalama
Copy link
Contributor Author

No problem at all! Prioritizing bug fixes and optimizations is important. Thanks for your time 🙏, and best of luck with the @nuxt/ui-pro release!

@benjamincanac
Copy link
Member

Still sorry for the delay, this will still not be part of the next 2.10.0 release, but it will be part of the 2.11 for sure!

@benjamincanac benjamincanac merged commit a35bfc7 into nuxt:dev Nov 16, 2023
1 of 2 checks passed
This was referenced Nov 21, 2023
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.

[Navigation] Breadcrumb
3 participants