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

Make linkValidation allow hash links #1143

Merged
merged 3 commits into from
Jul 20, 2016
Merged

Conversation

cruzanmo
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
New tests added? yes
Fixed tickets
License MIT

Description

Allows hash links, e.g. #fragment-identifier, when linkValidation is set to true. This was brought up in this discussion: #714

@coveralls
Copy link

coveralls commented Jul 13, 2016

Coverage Status

Coverage remained the same at 94.921% when pulling a357a67 on cruzanmo:allow-hash-link into d57e4b2 on yabwe:master.

link = editor.elements[0].querySelector('a');
expect(link).not.toBeNull();
expect(link.getAttribute('href')).toBe(validHashLink);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an extra space before and after this assertion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I will add spaces.

@coveralls
Copy link

coveralls commented Jul 19, 2016

Coverage Status

Coverage remained the same at 94.921% when pulling 319233f on cruzanmo:allow-hash-link into d57e4b2 on yabwe:master.

@@ -235,7 +235,8 @@
// Matches any alphabetical characters followed by ://
// Matches protocol relative "//"
// Matches common external protocols "mailto:" "tel:" "maps:"
var urlSchemeRegex = /^([a-z]+:)?\/\/|^(mailto|tel|maps):/i,
// Matches relative hash link, "#" followed by any character
var urlSchemeRegex = /^([a-z]+:)?\/\/|^(mailto|tel|maps):|^\#.+/i,
Copy link
Member

@nmielnik nmielnik Jul 19, 2016

Choose a reason for hiding this comment

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

Should this allow just plain # as a valid link? Right now it seems like it wouldn't allow that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, the hash should be omitted if there is no fragment identifier; however, you are correct that it is valid, so it should be allowed here.

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Coverage remained the same at 94.921% when pulling 635becb on cruzanmo:allow-hash-link into d57e4b2 on yabwe:master.

@nmielnik
Copy link
Member

Looks awesome @cruzanmo , thanks a ton!

@nmielnik nmielnik merged commit d880685 into yabwe:master Jul 20, 2016
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.

None yet

4 participants