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 #40

Closed
wants to merge 2 commits into from
Closed

feat: breadcrumb #40

wants to merge 2 commits into from

Conversation

zediogoribeiro
Copy link

@zediogoribeiro zediogoribeiro commented Oct 29, 2024

Changes

  • Add breadcrumb component
  • Update some minor/patch versions on package.json
  • Removed eslint-plugin-react-hooks since eslint-config-next already includes that lib

Captura de ecrã 2024-10-29, às 14 21 19

@zediogoribeiro zediogoribeiro marked this pull request as ready for review October 29, 2024 14:11
@zediogoribeiro zediogoribeiro force-pushed the breadcrumb branch 2 times, most recently from 0d2980b to 743a94f Compare October 29, 2024 14:17
@andrre-ls
Copy link
Contributor

Great work @zediogoribeiro!

Is there any use-case for either a Breadcrumb (container) with anything other than a BreadcrumbList as a child, or multiple BreadcrumbList? I ask because it seems like we're always doing:

<Breadcrumb>
    <BreadcrumbList>
        ...
    </BreadcrumbList>
</Breadcrumb>

and if the answer is "no", we maybe could drop BreadcrumbList.

@@ -1,6 +1,6 @@
{
"extends": ["next", "next/typescript", "plugin:react-hooks/recommended"],
"plugins": ["react-hooks"],
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Author

Choose a reason for hiding this comment

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

Because next already have inside react-hooks plugin inside

@pdrbrnd pdrbrnd closed this Jan 17, 2025
@pdrbrnd pdrbrnd deleted the breadcrumb branch January 17, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants