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

refactor: update SuiteSelectorSuiteLink with a and href to handle right-click #472

Merged

Conversation

nastyastavitskaya
Copy link
Collaborator

@nastyastavitskaya nastyastavitskaya commented Oct 23, 2024

Summary

  • To enhance UX, the links in the nav switcher are updated with a and href to handle opening in a new tab with the right-click.

Testing Plan

  • Was this tested locally? If not, explain why.
  • Run global navigation story with nav switcher. Confirm the right-click is now supporting an option to open link in a new tab.
Screenshot 2024-10-23 at 1 51 12 PM

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

@ilyusha
Copy link
Collaborator

ilyusha commented Oct 23, 2024

Review Forge

Click Here for AI Insights

Copy link

github-actions bot commented Oct 23, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-10-24 21:52 UTC

@gabyzif
Copy link
Contributor

gabyzif commented Oct 24, 2024

Looking great! However, I think we should prevent right-click functionality for the elements in a disabled state.

Copy link
Collaborator

@tibuurcio tibuurcio left a comment

Choose a reason for hiding this comment

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

lgtm! :)

@tibuurcio
Copy link
Collaborator

Looking great! However, I think we should prevent right-click functionality for the elements in a disabled state.

That's an interesting one... right-click is a native feature so I wouldn't prevent it but you're right @gabyzif we might want to remove the <a> if it's disabled 🤔

@nastyastavitskaya
Copy link
Collaborator Author

Looking great! However, I think we should prevent right-click functionality for the elements in a disabled state.

That's an interesting one... right-click is a native feature so I wouldn't prevent it but you're right @gabyzif we might want to remove the <a> if it's disabled 🤔

good point. I don't really like to mess up by rendering different html tags. I think the easiest would be to set the CSS rule pointer-events: none;. the downside for it is if we want any tooltip or something it won't work. but I think it should not be the case for this component

@nastyastavitskaya nastyastavitskaya merged commit a81adc0 into main Oct 24, 2024
9 checks passed
@nastyastavitskaya nastyastavitskaya deleted the refactor/update-suite-selector-to-handle-right-click branch October 24, 2024 21:52
@mparticle-automation
Copy link
Collaborator

🎉 This PR is included in version 1.34.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants