Skip to content

[docs-infra] Prevent link anchor when selecting text#41994

Merged
alexfauquette merged 6 commits intomui:nextfrom
alexfauquette:experiement-link
Jun 6, 2024
Merged

[docs-infra] Prevent link anchor when selecting text#41994
alexfauquette merged 6 commits intomui:nextfrom
alexfauquette:experiement-link

Conversation

@alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Apr 21, 2024

Fix #39350, solving #39603 (review)

The prevent default is weird, because if we use the documents.addEventlistner to trigger event.preventDefault() the link is still working 🙈
Did not found the why, so I just moved to the simplest approach: loop over links and add the event listener.

Screencast.from.16-05-2024.11.37.07.mp4

@alexfauquette alexfauquette added the scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). label Apr 21, 2024
@mui-bot
Copy link

mui-bot commented Apr 21, 2024

Netlify deploy preview

https://deploy-preview-41994--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 708c968

@alexfauquette alexfauquette marked this pull request as ready for review May 16, 2024 09:36
@alexfauquette alexfauquette requested a review from a team May 16, 2024 09:36
@alexfauquette alexfauquette changed the title [Draft] Try to prevent the link anchor when selecting [Draft] Prevent link anchor when selecting text May 17, 2024
alexfauquette and others added 2 commits June 5, 2024 15:55
Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
@alexfauquette alexfauquette merged commit d60a136 into mui:next Jun 6, 2024
@alexfauquette alexfauquette changed the title [Draft] Prevent link anchor when selecting text [docs-infra] Prevent link anchor when selecting text Jun 6, 2024
codeOpen: PropTypes.bool.isRequired,
codeVariant: PropTypes.string.isRequired,
copyButtonOnClick: PropTypes.object.isRequired,
copyButtonOnClick: PropTypes.func.isRequired,
Copy link
Member

Choose a reason for hiding this comment

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

Nice! I was about to open a PR to fix this prop type 👏

Copy link
Member Author

Choose a reason for hiding this comment

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

Some time I open my dev tools and spot warning errors 😁

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 9, 2024

Cool, the UX looks as good as it could be ✨

I noticed a couple of ways we can improve this: #42593.

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. design This is about UI or UX design, please involve a designer. labels Jun 9, 2024
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

design This is about UI or UX design, please involve a designer. scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[docs-infra] Make the whole header clickable

5 participants