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

fix(Menu): Pass onClick through even when rendered as a link #4845

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

dougmacknz
Copy link
Contributor

Why

No reason to block this, it's needed in some cases where you want to do something before navigating, like tracking. Best to still keep the element as a link in the DOM in this case.

What

Currently, onClick is not being passed through in the case that MenuItem has an href and renders a link

@dougmacknz dougmacknz requested a review from a team as a code owner July 18, 2024 05:00
Copy link

changeset-bot bot commented Jul 18, 2024

🦋 Changeset detected

Latest commit: 3219da5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@kaizen/components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Moskvyak
Moskvyak previously approved these changes Jul 18, 2024
Copy link
Contributor

github-actions bot commented Jul 18, 2024

✨ Here is your branch preview! ✨

Last updated for commit 3219da5: Empty commit

Copy link
Contributor

@mcwinter07 mcwinter07 left a comment

Choose a reason for hiding this comment

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

@dougmacknz I think Cass was saying if we rebase off main and force push to the branch this will (hopefully) pass chromatic. Holler if you want me to give that a crack :)

@mcwinter07
Copy link
Contributor

@dougmacknz I think Cass was saying if we rebase off main and force push to the branch this will (hopefully) pass chromatic. Holler if you want me to give that a crack :)

nvm, commented just as you did 😆

Copy link
Contributor

@mcwinter07 mcwinter07 left a comment

Choose a reason for hiding this comment

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

Looks hunky dory - I think the story flagged in chromatic wasn't picked up when it got merged last time so probs why its in the diff

@dougmacknz dougmacknz merged commit e30d286 into main Jul 18, 2024
17 checks passed
@dougmacknz dougmacknz deleted the allow-onclick-on-menu-item-href branch July 18, 2024 06:48
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.

3 participants