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 client-side routing in MenuItem component. (6.0) #19276

Merged
merged 2 commits into from
May 8, 2024

Conversation

dennisoelkers
Copy link
Member

Note: This is a backport of #19225 to 6.0.

Description

Motivation and Context

With the migration of the MenuItem component to Mantine, we introduced a regression leading to client-side routing (rendering a new page without a full page refresh) not working when an href prop is passed to the MenuItem component.

This PR is now fixing this by using the Link-component instead of a n a-element when the href prop is specified, is a relative URL and target/rel are not specified.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

* Fix client-side routing in `MenuItem` component.

* Adding changelog snippet.

* Simplify logic.

* Using `Link` component instead of `onClick`-handler.

* Cleaning up.
@dennisoelkers dennisoelkers merged commit 098c4e0 into 6.0 May 8, 2024
6 checks passed
@dennisoelkers dennisoelkers deleted the fix/client-side-routing-in-menu-item-6.0 branch May 8, 2024 06:04
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.

None yet

2 participants