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

Add body link fallback handler for in-app navigation #7627

Merged
merged 4 commits into from
Jan 25, 2022

Conversation

jryans
Copy link
Collaborator

@jryans jryans commented Jan 25, 2022

This adds a fallback click handler for all links in the message to ensure that links that were ignored as part of linkification (e.g. pills, links in message content) will still navigate in-app where possible.

This approach preserves a side benefit of #7453, which kept all link hrefs as matrix.to style links (so that they can be easily copied and shared).

Fixes element-hq/element-web#20715
Regressed by #7453


This PR currently has no changelog labels, so will not be included in changelogs.

Add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is plus X-Breaking-Change if it's a breaking change.

Preview: https://61f0086a29ac3143ccfbf492--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

This adds a fallback click handler for all links in the message to ensure that
links that were ignored as part of linkification (e.g. pills, links in message
content) will still navigate in-app where possible.

This approach preserves a side benefit of
#7453, which kept all link
hrefs as matrix.to style links (so that they can be easily copied and shared).

Fixes element-hq/element-web#20715
@jryans jryans requested a review from a team as a code owner January 25, 2022 13:48
Copy link
Contributor

@kerryarchibald kerryarchibald left a comment

Choose a reason for hiding this comment

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

Looks good, left some nits and a question.

src/components/views/messages/TextualBody.tsx Outdated Show resolved Hide resolved
* were ignored as part of linkification because they were already links
* to start with (e.g. pills, links in the content).
*/
private onBodyLinkClick = (e: MouseEvent): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure if they worked before, but what about ctrl + clicking links in messages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah hmm... IIRC, we have always forced in-app navigation (at least in recent times), but it does seem reasonable to support Ctrl/Cmd + Click to force a separate tab for those who want it. Since this is aimed at fixing a regression and that's effectively an enhancement, I'd say it's best to file an issue track that separately.

@jryans jryans merged commit fad65f9 into develop Jan 25, 2022
@jryans jryans deleted the jryans/in-app-links branch January 25, 2022 14:31
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.

In-app link navigation broke on Element Nightly and develop
2 participants