Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

🎨 Add basic router and wp link directive #83

Merged

Conversation

luisherranz
Copy link
Member

This PR is branched on top of #82.

This PR adds the remaining functionality: router + wp-link directive.

It's pretty much a copy/paste of the Query Loop block and Product Query block experiments.

@luisherranz luisherranz self-assigned this Oct 17, 2022
Base automatically changed from Add-basic-full-vdom-functionality to main-wp-directives-plugin October 17, 2022 14:28
@luisherranz luisherranz requested a review from DAreRodz October 17, 2022 14:43
@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Oct 17, 2022

Not sure if I'm doing something wrong but it seems this PR breaks the WP Admin. I have trouble navigating between pages and at some point, I am logged out automatically. I guess we will need to add something to prevent this from happening in the WP Admin?

EDIT: Oh, it is this code to test it out and it should be fixed once we add the wp-link attribute in PHP in #81?

Copy link
Collaborator

@DAreRodz DAreRodz left a comment

Choose a reason for hiding this comment

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

I haven't tested it, but the code looks great. 💯

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Oct 17, 2022

EDIT: Oh, it is this code to test it out and it should be fixed once we add the wp-link attribute in PHP in #81?

Yes, that was the issue I was facing. I have just merged #81 so I assume we can remove this code right? However, in that PR I am adding wp-link="true" and we are expecting something like wp-link="{ "prefetch": true }". Should we transform wp-link="true" into wp-link="{ "prefetch": true }" in the directive or in PHP?

Apart from that, I made the changes manually and it works great! 🎉 👏

@luisherranz
Copy link
Member Author

I've removed the manual wp-link injection and enqueued the scripts only on the frontend (my fault).

Should we transform wp-link="true" into wp-link="{ "prefetch": true }" in the directive or in PHP?

Yeah, but first we need to consider what prefetching strategy we want to enable here.

@luisherranz luisherranz merged commit c278c32 into main-wp-directives-plugin Oct 17, 2022
@luisherranz luisherranz deleted the basic-router-and-wp-link-directive branch October 17, 2022 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants