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

Try open matrix: links before showing clients #223

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Aug 24, 2021

Fixes #192

src/Link.js Outdated
return null;
}
const identifier = this.identifier.substring(1);
const suffix = this.eventId ? `/e/${this.eventId.substring(1)}` : "";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to url-encode event id and identifier here I think

const matrixUrl = this.link.toMatrixUrl()
if (matrixUrl) {
this.hidden = true;
setTimeout(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps pass setTimeout in as a view model option in case we want to unit test these one day

_tryLink() {
const matrixUrl = this._link.toMatrixUrl()
if (matrixUrl) {
this.tryingLink = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't you need an emitChange here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I assumed not, since this is called from the constructor, and the views are only created afterwards. I don't think any handlers can be registered at the time of the constructor call? Maybe I'm misunderstanding.

@DanilaFe
Copy link
Contributor

This is now out of date; I am working on getting this PR up to speed with the new designs.

@DanilaFe
Copy link
Contributor

Confusingly, Nheko (the only other native client) uses matrix: links as its deep link. This means that when trying to open Nheko when it's chosen as the "preferred" client, we will actually try to open literally any client (including web ones) that supports the scheme.

@ShadowJonathan
Copy link

I recommend adding "fixes #192" in the PR description, that way github will link this PR as a potential closer for that issue.

@bwindels bwindels marked this pull request as ready for review August 31, 2021 15:56
@deepbluev7
Copy link
Contributor

@DanilaFe I was just lazy and wanted an easy way to open a room in Nheko. Once this lands, we can probably just remove the open button for Nheko. :3

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.

Generate matrix scheme URIs
4 participants