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

Implement wp-context attribute directive#143

Merged
ockham merged 2 commits intomain-wp-directives-pluginfrom
add/wp-context-attribute-directive
Feb 9, 2023
Merged

Implement wp-context attribute directive#143
ockham merged 2 commits intomain-wp-directives-pluginfrom
add/wp-context-attribute-directive

Conversation

@ockham
Copy link
Copy Markdown
Collaborator

@ockham ockham commented Feb 1, 2023

Add the wp-context attribute directive (which we had previously only implemented as a tag directive).

While handling the closing tag will require quite a bit of complex logic, I now think that that logic should go into the wp_process_directives loop rather than the individual directive processor.

@ockham ockham self-assigned this Feb 1, 2023
@ockham ockham marked this pull request as ready for review February 1, 2023 11:15
@ockham ockham requested a review from a team February 1, 2023 11:17
Copy link
Copy Markdown
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

I don't know if the $tags->is_tag_closer() part is already available (I guess not, right?) but the code of the directive seems great to me. Pretty simple, actually 🙂

@ockham
Copy link
Copy Markdown
Collaborator Author

ockham commented Feb 9, 2023

Thank you for reviewing, @luisherranz!

I don't know if the $tags->is_tag_closer() part is already available (I guess not, right?) but the code of the directive seems great to me. Pretty simple, actually 🙂

FWIW, is_tag_closer() is already available in GB 😊

@ockham ockham merged commit cac7a1e into main-wp-directives-plugin Feb 9, 2023
@ockham ockham deleted the add/wp-context-attribute-directive branch February 9, 2023 09:48
ockham added a commit that referenced this pull request Feb 15, 2023
Some attribute directives (such as `wp-context`, see #143), need to be run not only on an opening tag (e.g. `<div wp-context="...">` but also on the matching closing one. (For `wp-context`, this is required to reset the context to the value that corresponds to the enclosing scope's context.)

This needs somewhat complex logic to keep track of nested tags. Consider e.g. the following example (used in this PR's unit test):

```html
<div>Example: 
	<div foo-test="abc">
		<img>
		<span>This is a test></span>
		<div>Here is a nested div</div>
	</div>
</div>
```

Here, we want to run the `foo-test` attribute directive processor on the closing `</div>` that matches the opening `<div foo-test="abc">` -- but not on any of the other two closing `</div>`s.

Rather than requiring individual directive processors to implement the required tracking of nested tags, we implement it in the `wp_process_directives` function.

To that end, this commit also moves it into a separate file and changes its signature a bit for better testability. Finally, we're adding a unit test to verify the behavior described above.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants