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

NavigationLink: make the URL dynamic for links to Posts #18345

Open
retrofox opened this issue Nov 6, 2019 · 17 comments · May be fixed by #46891
Open

NavigationLink: make the URL dynamic for links to Posts #18345

retrofox opened this issue Nov 6, 2019 · 17 comments · May be fixed by #46891
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@retrofox
Copy link
Contributor

retrofox commented Nov 6, 2019

The link URL should be generated dynamically, instead of storing the URL as an attribute.

Edit for clarity. To reproduce:

  1. Create a page, name it something
  2. Create a post
  3. In the post insert a navigation block
  4. In the block insert a link to the page you created
  5. Save and publish
  6. Check that your link links to the right page
  7. Go to the pages list in WP Admin
  8. Find your page and click the quick edit link that appears when you hover it
  9. Copy the slug
  10. Change the slug to lorem-ipsum
  11. Save
  12. Go to the page with the menu on the front end, refresh the page
  13. Hover the link: the URL should be the old one, not lorem-ipsum
  14. Clicking it should still take you to the correct page because WP handles redirects
  15. Create a new page and publish it
  16. Go to the pages list in WP Admin
  17. Find your new page and click the quick edit that appears when you hover it
  18. Replace the slug by pasting the one you copied earlier
  19. Save
  20. Go to the page with the menu on the front end, refresh the page
  21. Hover the link: the URL should be the old one, not lorem-ipsum
  22. Clicking it should take you to the wrong page because WP does not handle the redirect now
@jorgefilipecosta jorgefilipecosta added the [Block] Navigation Affects the Navigation Block label Nov 21, 2019
@obenland
Copy link
Member

obenland commented Jan 7, 2020

@getdave How big of a change do you think it would be to let navigationLink handle post objects?

@getdave getdave changed the title NavigationMenuLink: Make the URL dynamic NavigationMenuLink: make the URL dynamic for links to Posts Jan 8, 2020
@getdave
Copy link
Contributor

getdave commented Jan 8, 2020

@retrofox Can I clarify that you are suggesting that when you search for the Post and then select one of the options it should store a reference to that post (eg: ID) and then each time retrieve the URL from that object? But for all manually entered URLs it would continue to simply store the URL?

One thing that strikes me is that in any implementation we should be careful about ensuring that we're not coupling the Editor too tightly to WordPress.

I'll put this on my list to investigate tomorrow.

I'd also recommend we get some insight from @youknowriad as he and I were discussing something similar on his LinkControl API refactor PR.

@retrofox retrofox changed the title NavigationMenuLink: make the URL dynamic for links to Posts NavigationLink: make the URL dynamic for links to Posts Jan 8, 2020
@youknowriad
Copy link
Contributor

This feels related to the discussion we had here #19462 (comment)

basically LinkControl should be able to handle different "type" of links. Relative or not. The value should always provide the "URL" if needed (for example the link format in paragraph blocks will always store URLs) but the Navigation block could decide to store the "relative" link (couple id + type)

@getdave
Copy link
Contributor

getdave commented Jan 9, 2020

Thanks all.

Personally, I feel it would be useful to document what would be involved in this and what different scenarios we'd need to cover.

  • How would the process of dynamic link lookup work?
  • How would this be manifested in the UI?
  • Where would the data be stored and how?

@retrofox
Copy link
Contributor Author

retrofox commented Jan 9, 2020

Thanks, @getdave to taking over of this one. 👍 I think @youknowriad already brought a nice path to take here.

@mtias mtias added [Priority] High Used to indicate top priority items that need quick attention [Type] Task Issues or PRs that have been broken down into an individual action to take labels Mar 10, 2020
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Mar 20, 2020
@noisysocks noisysocks assigned noisysocks and unassigned draganescu Aug 26, 2020
@noisysocks noisysocks removed [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress labels Aug 27, 2020
@noisysocks
Copy link
Member

As part of this we should also set id and type when creating a Navigation block from an existing menu. See https://github.com/WordPress/gutenberg/pull/24846/files#r478837573.

@noisysocks
Copy link
Member

noisysocks commented Aug 28, 2020

To have compatibility with menu items, though, we may need to have three attributes which are used to set the dynamic URL. This parallels how menu items are stored in the database.

  • Type: post_type or taxonomy.
  • Object type / subtype: post, page, post_tag, category, etc.
  • Object ID / ID: The ID of the post, page, tag, category, etc.

With only type, it's impossible to link to custom post types and taxonomies. For example, it's not possible to tell from just type = 'book' whether 'book' is a post type or a taxonomy.

@mrfoxtalbot
Copy link

I am seeing that the current markup added for links looks as follows:

<a href="https://example.com/slug/" data-type="page" data-id="5573">Text</a>

I believe this is now solved, but please feel free to reopen if necessary @retrofox, @getdave

Thank you!

@draganescu
Copy link
Contributor

Currently the navigation link block is storing the link in an attribute defined as:

"url": {
	"type": "string"
},

This means that when the page is rendered the link to the page or post is hardcoded in the block's markup. Therefore if the URL changes the link will be stale.

@draganescu draganescu reopened this Oct 3, 2022
@mrfoxtalbot
Copy link

Oh... 😬 I misunderstood the issue, apologies and thank you for reopening it @draganescu!

@getdave
Copy link
Contributor

getdave commented Oct 4, 2022

To quote Riad here.

The value should always provide the "URL" if needed (for example the link format in paragraph blocks will always store URLs) but the Navigation block could decide to store the "relative" link (couple id + type)

So what we can look to do is have the Nav Link block avoid storing the URL and instead store the id + type reference. That way we can look up the correct URL in the Editor at runtime. Ditto on the front end.

Currently

  • Editor: the Nav Link block creates a link value object which it passes to <LinkControl>. This link object contains the url value. We could dynamically lookup the url value based on id and append to the link object.
  • Front of site: the Nav Link block uses the url from the stored attributes. Ironically it actually already looks up the $post object based on the id attribute but the data isn't used. We could change this to allow for that to dynamically look up the url.

@draganescu
Copy link
Contributor

I tried the above once many months ago and it requires significant updates to how link control works. It is the way tho' :) I just wanted to mention it's complicated by how LinkControl works.

@getdave
Copy link
Contributor

getdave commented Oct 11, 2022

it's complicated by how LinkControl works.

Thanks for the context Andrei. We'll have to dive into LinkControl - luckily I'm pretty familiar with it.

@getdave
Copy link
Contributor

getdave commented Mar 15, 2023

Related #32282

@warudin
Copy link

warudin commented Aug 25, 2023

Just walked into this issue with our first Block Theme. Is there currently no way to make the navigation block store links in a relative way instead of the url?

The classic menu interface saved items in a relative way, right?

I am surprised that this issue is not generating much more attention.

@draganescu
Copy link
Contributor

@warudin yes it is a very big issue, but it's also big in the sense it requires a lot of connected parts to change. Maybe the work on #50891 will include some updates to make this more reachable.

@getdave
Copy link
Contributor

getdave commented Nov 20, 2024

With "recent" advancements to the Link UI I think we could be in a good place to address this. Here's how:

  • update the Link UI to allow for disabling the URL field.
  • make the Nav Link block only store the ID of the entity.
  • make the Nav Link block dynamically fetch the correct URL
  • disable the URL field in the Link UI used int he Nav Link block.

The UX to figure out is:

  • how we best communicate that the menu item is linking to an entity
  • how we allow users to "unlink" the entity and search for a new link (like you can currently)
  • how we allow users to append to the end of the URL (e.g. internal #anchor links).

Some initial work in #46891 but this would need rebasing and updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
10 participants