Fix wp-class and introduce the evaluate function#127
Fix wp-class and introduce the evaluate function#127luisherranz merged 12 commits intomain-wp-directives-pluginfrom
Conversation
DAreRodz
left a comment
There was a problem hiding this comment.
Looks good to me, @luisherranz! 👌
PS: I would have added some e2e tests for class names contained inside others (e.g., "foo foo-bar") or just part of a class name (e.g. foo inside "foo-bar foo-baz") to ensure that the regexp only handle whole class names. This is something you mentioned in the video, so I was expecting these kinds of tests to be included. 🙂
I'll add them before merging. Thanks, David! |
michalczaplinski
left a comment
There was a problem hiding this comment.
Looks 💯 ! TIL about the Playwright VSCode extension!
Thank @luisherranz! I think it's enough. That test would have been just to ensure that the directive doesn't mess with other classes containing the one defined for the directive, even when that class is not there yet. But I guess that is already covered with the first test. 😊 |
|
Merged then 🙂 |
What
I improved the
wp-classimplementation because it was very basic. My idea was also to add unit tests, but instead I added more e2e tests. I also took the opportunity to improve the logic that resolves thepath.Why
Because the implementation was so basic that it could fail with very simple cases.
How
For
wp-classI have introduced regular expressions. For the e2e tests I followed what I did in my previous pull request. And for the logic that resolvespath, I have introduced a new functionevaluate.I did a video to explain in detail each change:
https://www.loom.com/share/2fd4cbd8963f416da49b71c484d68b65