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

core(canonical): add 'not recommended' to relative url string #12479

Closed
wants to merge 1 commit into from

Conversation

techguysid
Copy link

Summary
Adds a not recommended flag in relative URLs description.

Related Issues/PRs
Ref #12463

Copy link
Member

@adamraine adamraine left a comment

Choose a reason for hiding this comment

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

You can run yarn update:sample-json to propagate changes to language files.

@@ -32,7 +32,7 @@ const UIStrings = {
* @description Explanatory message stating that there was a failure in an audit caused by a URL being relative instead of absolute.
* @example {https://example.com/} url
* */
explanationRelative: 'Relative URL ({url})',
explanationRelative: 'Relative URL ({url}) (not recommended)',
Copy link
Member

Choose a reason for hiding this comment

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

Double parenthesis feels weird.

Suggested change
explanationRelative: 'Relative URL ({url}) (not recommended)',
explanationRelative: 'Relative URL not recommended ({url})',

@connorjclark connorjclark changed the title core: add not-recommended in relative url description core(canonical): add 'not recommended' to relative url string May 14, 2021
@@ -32,7 +32,7 @@ const UIStrings = {
* @description Explanatory message stating that there was a failure in an audit caused by a URL being relative instead of absolute.
* @example {https://example.com/} url
* */
explanationRelative: 'Relative URL ({url})',
explanationRelative: 'Relative URL ({url}) (not recommended)',
Copy link
Member

Choose a reason for hiding this comment

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

an alternative... we communicate it's not absolute. that's what we want.

Suggested change
explanationRelative: 'Relative URL ({url}) (not recommended)',
explanationRelative: 'Is not an absolute URL ({url})',

Copy link
Member

Choose a reason for hiding this comment

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

#12676 adopted my suggestion, so that works fine for me. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants