Add options for minimal links and auto-prefixing http protocol #727

SDRACK · 2018-03-01T10:17:32Z

Two options added: autoPrefixHttpProtocol and minimalLinks.

minimalLinks slims down the link overlay to use only text and url.

autoPrefixHttpProtocol adjust links without a protocol. Those with a protocol, anchors and relative links are left alone. Anything else if prepended with http:// or mailto: as appropriate.

Both new options default to false.

N.B. ES6 methods used for string interpolation. Can adjust if required.

Alex-D · 2018-03-01T18:22:35Z

src/trumbowyg.js

+ const VALID_LINK_PREFIX = /^([a-z][-+.a-z0-9]*:|\/|#)/i;
+ if(VALID_LINK_PREFIX.test(url)) { return url; }
+
+ return url.includes("@") ? `mailto:${url}` : `http://${url}`;


That's wrong, you can use @ in HTTP url to put some user/password:

[//[user[:password]@]host[:port]][/path][?query][#fragment]

Alex-D · 2018-03-01T18:23:48Z

Thanks for your Pull Request :)

Yep, do not use ES6 since I did not use babel and want to be ES5-compatible.

SDRACK · 2018-03-02T10:58:59Z

Cheers for the feedback - I'll update the ES6 bits and implement a regex to distinguish email addresses / urls.

Be back with you soon 👍

Simple regex used for testing whether link input is an email rather than a url.

…velop

forelabs · 2018-03-03T15:33:57Z

I suggest to rename it to autoPrefixProtocol and have an urlProtocol option which defaults to https, but can be set to e.g. Http. Https will be the new Standart since chrome will also mark websites as untrusted soon when they use http.

…tring

SDRACK · 2018-03-05T11:44:03Z

Thanks, sounds sensible. I've updated the option name. Under the new setup, users can provide the following options to urlProtocol:

false (default)
true, 'https' or 'https://' => 'https://'
'http' or 'http://' => 'http://'

Anything else will be ignored.

Let me know what you think - appreciate your work on this!

Alex-D

Please stick to the eslint configuration to keep coherence :)

Alex-D · 2018-03-05T14:03:29Z

src/trumbowyg.js

+
+ if(typeof(protocol) === 'boolean') {
+ return 'https://'
+ } else if(typeof(protocol) === 'string') {


Don't need else as we always manage all others cases with the previous if

Alex-D · 2018-03-05T14:04:11Z

src/trumbowyg.js

+
+ if(!protocol) { return; }
+
+ if(typeof(protocol) === 'boolean') {


if (typeof protocol !== 'string') {

forelabs · 2018-03-05T14:47:24Z

src/trumbowyg.js

- target: {
- label: t.lang.target,
- value: target
+ value: new XMLSerializer().serializeToString(documentSelection.getRangeAt(0).cloneContents())


this one is out of date and should stay text

forelabs

from my side this looks fine now

Alex-D · 2018-03-13T10:44:48Z

src/trumbowyg.js

- }
+ if(typeof(protocol) !== 'string') { return 'https://'; }
+ if(/^https?:\/\/$/.test(protocol)) { return protocol; }
+ if(/^https?$/.test(protocol)) { return protocol + '://'; }


That's a lot of code to just get https:// or http:// :/

What about others protocols? If I want put some ftp:// or whatever?

Thanks @Alex-D.

Would you prefer to allow any string value in the option, and append :// if it's missing?

So:

a true value defaults to https://

a string without the correct suffix has :// appended to it

a string with the correct suffix is prepended as is

yep I think it could be cool :)

SDRACK · 2018-03-15T10:55:05Z

Morning @Alex-D - that's the setupUrlPrefix function shortened and set to allow any string as said prefix. For easy reference, here's the updated code:

        setupUrlPrefix: function() {
            var protocol = this.o.urlProtocol;
            if(!protocol) { return; }

            if(typeof(protocol) !== 'string') { return 'https://'; }
            return /:\/\/$/.test(protocol) ? protocol : protocol + '://';
        }

How does that look to you?

Alex-D · 2018-03-15T13:36:54Z

LGTM :)

Could you update the doc about this new option? It's somewhere here:

Thanks!

SDRACK · 2018-03-15T15:43:56Z

Fantastic - thanks @Alex-D. Really appreciate all your work with me on this.

Let me know what you think of the updated docs and I'll edit if needed.

One thing to note: I've put 2.9.4 as the version added as I'm not sure of your process for iterating this. Let me know if that needs tweaking.

SDRACK · 2018-03-28T13:54:11Z

@Alex-D - please don't consider this a chase, but I'm wondering what your process is for introducing new features such as these, and when you're next likely to do so?

Hope I'm not being a pain asking this :)

Alex-D · 2018-03-28T21:47:40Z

Just taking time to run this on my computer, then merge other MRs, release a new version, test it, etc.

So I need to take couple of hours ^^'

Sorry if it's long...

SDRACK · 2018-03-29T09:31:21Z

Thanks, that clears things up perfectly. It was more a case of establishing whether this would be deployed shortly or as part of a larger release way down the line.
Thanks again for your work on this.

Alex-D · 2018-04-09T01:55:46Z

Thanks! :)

Was inserted in v2.10.0 / Alex-D#727.

jorrit · 2018-04-19T12:34:10Z

This PR (and release 2.10.0) broke ES5 browsers/minifiers because of the use of const.

See #764.

SDRACK and others added 7 commits January 30, 2018 15:24

Add options to prefix links with protocol and show minimal link popup

a4d97c6

Update prefix option name to reflect 'http' protocol usage

7b5777b

Scope minimalLinks option

9a698cb

Update prependHttpPrefix to allow relative links and anchors

2658e0e

Merge 2.9.5

89e3b3a

Correct code indentation

535a28f

Revert to correct version

fc2bf7a

Alex-D requested changes Mar 1, 2018

View reviewed changes

SDRACK added 2 commits March 2, 2018 11:52

Allow URLs to contain '@'; remove ES6 code.

c0ad1cb

Simple regex used for testing whether link input is an email rather than a url.

Merge branch 'develop' of https://github.com/SDRACK/Trumbowyg into de…

07e85ca

…velop

Steve Rackham added 2 commits March 5, 2018 11:30

Change url prefix option to urlProtocol; allow this to take bool or s…

7334fbe

…tring

Switch to use regex.test for consistency

5cfecb2

Alex-D requested changes Mar 5, 2018

View reviewed changes

forelabs suggested changes Mar 5, 2018

View reviewed changes

Update to improve urlPrefix setup

8a0748f

forelabs approved these changes Mar 12, 2018

View reviewed changes

Alex-D requested changes Mar 13, 2018

View reviewed changes

Shorten setupUrlPrefix; allow any string as prefix

8463bff

Update docs to cover urlProtocol and minimalLinks options

1b35805

Alex-D approved these changes Apr 9, 2018

View reviewed changes

Alex-D merged commit 2d4b8c1 into Alex-D:develop Apr 9, 2018

jorrit added a commit to jorrit/Trumbowyg that referenced this pull request Apr 19, 2018

trumbowyg.js: Remove ES6 code

ff508c0

Was inserted in v2.10.0 / Alex-D#727.

jorrit mentioned this pull request Apr 19, 2018

trumbowyg.js: Remove ES6 code #764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add options for minimal links and auto-prefixing http protocol #727

Add options for minimal links and auto-prefixing http protocol #727

SDRACK commented Mar 1, 2018

Alex-D Mar 1, 2018

Alex-D commented Mar 1, 2018 •

edited

Loading

SDRACK commented Mar 2, 2018

forelabs commented Mar 3, 2018

SDRACK commented Mar 5, 2018

Alex-D left a comment

Alex-D Mar 5, 2018

Alex-D Mar 5, 2018

forelabs Mar 5, 2018

forelabs left a comment

Alex-D Mar 13, 2018

Alex-D Mar 13, 2018

SDRACK Mar 13, 2018

Alex-D Mar 14, 2018

SDRACK commented Mar 15, 2018

Alex-D commented Mar 15, 2018 •

edited

Loading

SDRACK commented Mar 15, 2018

SDRACK commented Mar 28, 2018

Alex-D commented Mar 28, 2018

SDRACK commented Mar 29, 2018 •

edited

Loading

Alex-D commented Apr 9, 2018

jorrit commented Apr 19, 2018


		if(!protocol) { return; }

		if(typeof(protocol) === 'boolean') {

Add options for minimal links and auto-prefixing http protocol #727

Add options for minimal links and auto-prefixing http protocol #727

Conversation

SDRACK commented Mar 1, 2018

Choose a reason for hiding this comment

Alex-D commented Mar 1, 2018 • edited Loading

SDRACK commented Mar 2, 2018

forelabs commented Mar 3, 2018

SDRACK commented Mar 5, 2018

Alex-D left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forelabs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SDRACK commented Mar 15, 2018

Alex-D commented Mar 15, 2018 • edited Loading

SDRACK commented Mar 15, 2018

SDRACK commented Mar 28, 2018

Alex-D commented Mar 28, 2018

SDRACK commented Mar 29, 2018 • edited Loading

Alex-D commented Apr 9, 2018

jorrit commented Apr 19, 2018

Alex-D commented Mar 1, 2018 •

edited

Loading

Alex-D commented Mar 15, 2018 •

edited

Loading

SDRACK commented Mar 29, 2018 •

edited

Loading