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.

Sign up for GitHub

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

Jump to bottom

Only autolink URLs containing protocol #2754

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Only autolink URLs containing protocol #2754

merged 1 commit into from
Jul 26, 2022

Conversation

Raudius
Copy link
Contributor

@Raudius Raudius commented Jul 25, 2022

vinicius73
vinicius73 requested changes Jul 25, 2022
View reviewed changes
Copy link
Member

@vinicius73 vinicius73 left a comment

Choose a reason for hiding this comment

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

LGTM;
But tests should be updated.
https://github.com/nextcloud/text/runs/7501209373?check_suite_focus=true

text/cypress/e2e/links.spec.js

Lines 43 to 55 in 0b8750d

it('whithout protocol', () => {
cy.getContent()
.type('google.com')
cy.getContent()
.find('a[href*="google.com"]')
.click({ force: true })
cy.get('@winOpen')
.should('have.been.calledOnce')
.should('have.been.calledWith', 'http://google.com/')
})
})

@susnux
Copy link
Contributor

susnux commented Jul 26, 2022

I guess restricting autolink to http(s) is fine, so lgtm after fixing the tests.

@Raudius Raudius force-pushed the enh/only_autolink_with_protocol branch 2 times, most recently from 86819a9 to b2a4705 Compare July 26, 2022 09:03
susnux
susnux requested changes Jul 26, 2022
View reviewed changes
@Raudius Raudius force-pushed the enh/only_autolink_with_protocol branch 2 times, most recently from 140cbea to 7ccc6c8 Compare July 26, 2022 10:43
@Raudius
Copy link
Contributor Author

Raudius commented Jul 26, 2022

/compile amend

@nextcloud-command nextcloud-command force-pushed the enh/only_autolink_with_protocol branch from 7ccc6c8 to ccf6f09 Compare July 26, 2022 10:55
@Raudius
Only autolink URLs containing protocol
32f0d22 
Signed-off-by: Raul <raul@nextcloud.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@Raudius Raudius force-pushed the enh/only_autolink_with_protocol branch from ccf6f09 to 32f0d22 Compare July 26, 2022 12:09
@juliusknorr juliusknorr added bug Something isn't working 4. to release labels Jul 26, 2022
@juliusknorr juliusknorr merged commit bd08124 into master Jul 26, 2022
@juliusknorr juliusknorr deleted the enh/only_autolink_with_protocol branch July 26, 2022 15:37
@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinicius73 vinicius73 vinicius73 approved these changes

@susnux susnux susnux approved these changes

@juliusknorr juliusknorr juliusknorr approved these changes

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud

Assignees
No one assigned
Labels
4. to release bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: Autolink feature should be optional
4 participants
@Raudius @susnux @vinicius73 @juliusknorr