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

Remove protocol in LinkControl suggestiosn #21279

Closed
wants to merge 1 commit into from

Conversation

draganescu
Copy link
Contributor

Description

Closes #19670. Props @sainthkh, copied test from #20350

How has this been tested?

Tested locally by:

  1. Make sure you have some posts and pages
  2. Add a post
  3. Add a navigation block
  4. Add a navigation item and search for something
  5. The internal links for suggested posts or pages should not display protocol
  6. The links should be saved with protocol

Types of changes

Non breaking change to display of links in LinkControl suggestion list.

@github-actions
Copy link

Size Change: +25 B (0%)

Total Size: 863 kB

Filename Size Change
build/block-editor/index.js 102 kB +25 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.44 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 7.21 kB 0 B
build/block-library/editor.css 7.21 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.5 kB 0 B
build/block-library/style.css 7.51 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.2 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.26 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 92.3 kB 0 B
build/edit-post/style-rtl.css 8.35 kB 0 B
build/edit-post/style.css 8.34 kB 0 B
build/edit-site/index.js 8.65 kB 0 B
build/edit-site/style-rtl.css 3.46 kB 0 B
build/edit-site/style.css 3.46 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.57 kB 0 B
build/edit-widgets/style.css 2.57 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.38 kB 0 B
build/editor/style.css 3.38 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.92 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.31 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 781 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@aduth
Copy link
Member

aduth commented Apr 1, 2020

Can you elaborate on how this differs from @sainthkh's work in #20350, and for what purpose? Could those have been suggested as review remarks on #20350 instead?

@draganescu
Copy link
Contributor Author

Hey @aduth it's just an alternative PR which is doing a simpler job, exclusively removing the protocol with replace. This can be added as a review there of course :)

@sainthkh
Copy link
Contributor

sainthkh commented Apr 2, 2020

Then, what should we do?

@aduth
Copy link
Member

aduth commented Apr 2, 2020

If it's intended as a distinct implementation proposal from #20350, I think that's fine to be a separate pull request. I might suggest to try to incorporate co-authorship of the commit to ensure @sainthkh is properly credited for the test implementation.

Generally, I'd prefer the approach of #20350 in its reuse of @wordpress/url utilities. Presumably filterURLForDisplay is intended to serve these sorts of use-cases of presenting a URL somewhere in the UI. If there's specifics about how it deals with www. that we need to make decisions on, I think we can make those decisions, and consider enhancing the utilities to support additional options as needed. One of the nice things about using filterURLForDisplay is that it consolidates the implementation of how we treat URLs, so that at a point where we decide to trim additional protocols (like ftp:), this will take effect universally. This would not be the case if we have duplicate implementations throughout the code.

@draganescu
Copy link
Contributor Author

I will close this based on @aduth review of the filterURLForDisplay approach in #20350. Will move there with any comments.

@draganescu draganescu closed this Apr 2, 2020
@aristath aristath deleted the add/remove-protocol-in-suggestion-link branch November 10, 2020 14:28
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.

LinkControl - remove URL protocol from Entity (only!) search results
3 participants