Add wp-show directive attribute#156
Add wp-show directive attribute#156SantosGuillamot merged 4 commits intomain-wp-directives-pluginfrom
wp-show directive attribute#156Conversation
luisherranz
left a comment
luisherranz
left a comment
There was a problem hiding this comment.
The code looks fine to me, but would you mind adding some e2e tests? Thanks!! 🙏
Sure, I'll do that 🙂 |
|
I started adding some tests, but something is not working as expected. It seems the HTML generated is not correct, and I don't know why it seems to be passing the attributes to the next HTML tag. I'll review tomorrow in more detail why that is happening. |
|
I've added more tests to check the toggling and that it works with context as well -> Commit. In this other commit, @luisherranz fixed the issue I mention here. However, we have to decide What should be the HTML structure returned when wp-show is false? before merging this PR. The current approach is working, but if we go with the other alternative, the problem mentioned still persists. |
|
Let's merge this as it is and if we decide to go with the opposite approach, we'll change it. I'll investigate the bug a bit and open another issue, as it's not related to this PR (it's a general bug). |
2 participants
This pull request aims to replicate the behavior of the
wp-showdirective tag (component), but using a directive attribute instead.It's true that in terms of functionality, the directive tag should be enough for now, but both approaches should be available. Especially if we end up removing directive tags (components) from the proposal.
In the movies demo, we will use this directive, so it would be great if we could just use directive tags.