Skip to content

Use Discord message links in Quoter#40

Open
bb010g wants to merge 1 commit intosamogot:masterfrom
bb010g:quoter-native-links
Open

Use Discord message links in Quoter#40
bb010g wants to merge 1 commit intosamogot:masterfrom
bb010g:quoter-native-links

Conversation

@bb010g
Copy link
Copy Markdown

@bb010g bb010g commented Oct 1, 2018

I think this covers everything backwards-compat wise, if you care about that. All the whitespaces changes are just from yarn run gulp, so you may want to view the diff with --ignore-space-change.


This change is Reviewable

@samogot
Copy link
Copy Markdown
Owner

samogot commented Oct 1, 2018

Last time I checked this links opened as external links in browser (without plugin installed). Can desktop clients recognize them now?

@bb010g
Copy link
Copy Markdown
Author

bb010g commented Oct 1, 2018

No, and I realized that my regex was a bit messed up, so fixing that now. This would still be an improvement on Android (where you can tell the link to open in the app and it works), and the plugin can force navigation like it did before until a better way is figured out.

@zerebos
Copy link
Copy Markdown
Collaborator

zerebos commented Oct 1, 2018

I addressed this in the server Samo. This doesn't really add much since it wouldn't work for other users on desktop which is the main feature here. Those links are opened properly if used in the markdown sections of the embed instead of as the title or author url however. So we could add an option to have an additional embed field with name Reference Link with the body of the field being the discord link.

@bb010g bb010g force-pushed the quoter-native-links branch from 183ca07 to 1cf7f30 Compare October 1, 2018 23:38
@bb010g
Copy link
Copy Markdown
Author

bb010g commented Oct 1, 2018

Ok, regexes are fixed and it actually jumps now with the plugin. I like the body embed idea for people who don't have this installed.

@bb010g bb010g force-pushed the quoter-native-links branch from 1cf7f30 to f8cede9 Compare October 1, 2018 23:42
@zerebos
Copy link
Copy Markdown
Collaborator

zerebos commented Oct 1, 2018

I have that mostly implemented, but I didn't want to add such a feature until Samogot came back and we could discuss it

@bb010g
Copy link
Copy Markdown
Author

bb010g commented Oct 2, 2018

Relevant verified bug for this behavior: https://trello.com/c/WD5FyVBu/2124-jump-urls-dont-work-if-masked

@ajayyy
Copy link
Copy Markdown

ajayyy commented Feb 1, 2019

The links seem to work on desktop now and web

@ajayyy
Copy link
Copy Markdown

ajayyy commented Mar 6, 2019

MERGABLE. This works perfectly

@Lypheo
Copy link
Copy Markdown

Lypheo commented Sep 16, 2019

Any update on this?

@ajayyy
Copy link
Copy Markdown

ajayyy commented Sep 18, 2019

@Lypheo it works completely fine. I run this version on my client and the links work.

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.

5 participants