Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Make pill @ and matrix.to links behave the same #7559

Closed
wants to merge 11 commits into from

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Jan 17, 2022

Another go at: #7550
The idea is to convert all links to matrix.to links.
This sadly breaks pills since they are expecting a local url. So we change all the pill related code to use and expect a matrix to permalink as well.

I wont bother about the tests until we decide, that this is actually the right approach


Here's what your changelog entry will look like:

✨ Features

  • Make pill @ and matrix.to links behave the same (#7559). Contributed by @toger5.

Preview: https://61e7114c58384814d8cb5ac8--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.

@toger5 toger5 requested a review from a team as a code owner January 17, 2022 15:45
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

otherwise seems fine - just need to fix the tests.

src/utils/permalinks/Permalinks.ts Outdated Show resolved Hide resolved
@toger5 toger5 requested a review from turt2live January 18, 2022 19:42
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I think this is sane

@jryans jryans changed the title make pill @ and matrix.to links behave the same Make pill @ and matrix.to links behave the same Jan 19, 2022
@jryans jryans added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Jan 19, 2022
@toger5
Copy link
Contributor Author

toger5 commented Jan 20, 2022

This is not idea:

if (transformed !== href || // for matrix symbols e.g. @user:server.tdl
                    decodeURIComponent(href).match(ELEMENT_URL_PATTERN) || // for https:vector|riot...
                    decodeURIComponent(href).match(MATRIXTO_URL_PATTERN) // for matrix.to
                ) {

but it gets superseded by #7453 anyways.
If we can get #7453 mergeable quick enough it might be best to just skit this pr?

@toger5
Copy link
Contributor Author

toger5 commented Jan 20, 2022

this review: #7559 (review)
was also applied to #7453

@jryans
Copy link
Collaborator

jryans commented Jan 20, 2022

If we can get #7453 mergeable quick enough it might be best to just skit this pr?

Sure, seems fine to me.

@toger5
Copy link
Contributor Author

toger5 commented Jan 20, 2022

#7453 is ready to merge so closing this.

@toger5 toger5 closed this Jan 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants