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

Open in new tab option, fragments, uri schemes, and fixes #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nickolasjadams
Copy link

Url checking

I noticed that when you don't add a protocol to a URL like this
www.hello.world
the URL added to the editor attempts to reference it like this
file:///path/to/project/www.hello.world

So I took a look at how to require http or https and noticed
Utils.isUrl() is not working properly in another way
due to the regex.

This is passing the regex
www.0.♪┏(・o・)┛♪

This is failing
http://wwwuzzzuppp.com

I've updated the regex to fix the regex issue and require the protocol.
Adding links should be done through the API or absolute URLs.


closes #8
Impossible to disable server search Issue #8
#8

isServerEnabled method calls lacked parenthesis in index.js. It's
now possible to disable the server calls by removing the endpoint
in the config.


closes #6
Support mailto: URL schema #6
#6

I've found myself wishing to add different types of hrefs and
noticed this issue.

I've added 3 URI schemes to check against in Utils.isUrl()

view-source:
mailto:
tel:

It now checks for a URI Scheme and then applies an appropriate
regex to test the remaining URL.

The email regex uses control characters so disabled no-control-regex in .eslintrc


Url fragments are not recognized as URLs.

Utils.isUrl() now also checks if your string starts with "#"

so you can do this <a href="#lower_in_the_document">

Which will work great with this plugin.
https://github.com/Aleksst95/header-with-anchor


Can't open links in a new tab.

A checkbox has been added to provide the user an option to open the URL in a new tab.
You can tab and shift-tab back and forth between the 2 inputs when adding URLs.
Clicking the datawrapper will let you know if the link opens in a new tab or the same tab.


Finally, I've fixed a couple of warnings that were shown when running the linter.

Url checking

I noticed that when you don't add a protocol to a url like this
www.hello.world
the url added to the editor attempts to reference a file
file:///path/to/project/www.hello.world

So I took a look at how to require http or https and noticed
Utils.isUrl() is not working properly in another way
due to the regex.

This is passing the regex
www.0.♪┏(・o・)┛♪

This is failing
http://wwwuzzzuppp.com

I've updated the regex to fix the regex issue and require the protocol.
Adding links should be done through the api or absolute urls.

---

closes editor-js#8
Impossible to disable server search Issue editor-js#8
editor-js#8

isServerEnabled method calls lacked parenthesis in index.js. It's
now possible to disable the server calls by removing the endpoint
in the config.

---

closes editor-js#6
Support mailto: URL schema editor-js#6
editor-js#6

I've found myself wishing to to add different tyes of hrefs and
noticed this issue.

I've added 3 URI schemes to check against in Utils.isUrl()

view-source:
mailto:
tel:

It now checks for a URI Scheme and then applies an appropriate
regex to test the remaining url.

The email regex uses control characters so disabled no-control-regex in .eslintrc

---

Url fragments not recongnized as urls.

Utils.isUrl() now also checks if your string starts with "#"

so you can do this `<a href="#lower_in_the_document">`

Which will work great with this plugin.
https://github.com/Aleksst95/header-with-anchor

---

Can't open links in a new tab.

A checkbox has been added to provide the user an option to open the url in a new tab.
You can tab and shift-tab back and forth between the 2 inputs when adding urls.
Clicking the datawrapper will let you know if the link opens in a new tab or the same tab.

---

Finally, I've fixed a couple warnings that were shown when running the linter.
@quaidesbalises
Copy link

What news about those features ?

@nickolasjadams
Copy link
Author

What news about those features ?

I'd love to know as well :)

@CodyPChristian
Copy link

@nickolasjadams This is great hopefully this PR can get merged already. @talyguryn @neSpecc @robonetphy

@neSpecc
Copy link
Contributor

neSpecc commented Sep 6, 2023

It's cool that you made several improvements in this tool, but not so cool that you've made them in a single PR. Next time it's better to separate changes on small atomic PRs which are easy to understand, review, and test.

@nickolasjadams
Copy link
Author

It's cool that you made several improvements in this tool, but not so cool that you've made them in a single PR. Next time it's better to separate changes on small atomic PRs which are easy to understand, review, and test.

Sorry about that. I worried about all this in one branch after I put all the work in.

I hoped documenting the PR well, and replying to any questions would help enough.

@nickolasjadams
Copy link
Author

@neSpecc @CodyPChristian @quaidesbalises
I've began breaking this PR into smaller ones.

The first one is here #11.

Should be a lot easier to test. Let me know if you have any feedback.

Thanks!

@balex25
Copy link

balex25 commented Nov 20, 2023

Any update on this, please?

@nickolasjadams
Copy link
Author

Any update on this, please?

It's likely not going to be merged. I've continued to keep my fork up to date. It's on npm if you want to take advantage of my fixes.

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.

Impossible to disable server search Support mailto: URL schema
5 participants