assert_dom :text collapses whitespace
- Add macos to supported platforms in Gemfile.lock
- Add failing test for assert_dom collapsing whitespace
- Collapse whitespace from :text but not :html
Fixes #121
This PR is my preferred solution over #122.
Instead of making use of a :strict operator to determine whether or not to collapse excess whitespace as the browser does, we instead use the existing :html equality operator to match for text with all whitespace included. The :text equality operator is then optimised to collapse all whitespace.
In my opinion, :text should match what would be returned by Element.innerText in javascript land, in that it does not include the excess whitespace. Element.innerHTML on the other hand, does include all of the whitespace that was in the document which I think is what the :html operator should match (which it does already) and so :html could instead be used as a :strict parameter.
The only difference between what Element.innerText and the updated :text equality operator does is that Element.innerText replaces <br> tags with \n, whereas our :text operator just removes them without replacing them. But this is out of our control as it is Nokogiri that does this. Regardless, this is my preferred solution but I made the two PRs since the original issue I raised specifically mentions a strict parameter.
There seems to be an issue with the CI workflows related to the Logger gem. I'll fix it on main and then you can rebase this and #122.
OK, main is green, so please rebase when you have a chance.
So to confirm, you won't both PRs merged, not just this one? I made the two PRs so that there were a couple of options for how this would be implemented, with the intention that only one would get chosen. Let me know what you think is best.
I should probably also update the commented docs just before the assert_dom definition to make the updated behaviour of :text and :html a little more clear.
So to confirm, you won't both PRs merged, not just this one
I just want whatever you want to submit rebased so I can see if the tests pass, please! Or let me know if you want me to do it, that's fine, too.
I'm sorry I've been busy this week, but in general I want to see:
- green CI (tests passing)
- then I will look at the code and review it
- then we can make a decision about merging
and we're still on that first item until the branch is rebased on the current main. I hope that makes sense and clarifies? 🙏
Ahhh right sorry for some reason I read "rebase" as merge the pull request. I'll rebase now.
@flavorjones Rebased and tests and ci is passing
@flavorjones just following up on this
👋🏻 just wanted to share that this change broke the ViewComponent test suite: https://github.com/ViewComponent/view_component/actions/runs/15148835486/job/42590881883?pr=2309.
The fix was pretty minor (https://github.com/ViewComponent/view_component/pull/2309/files#diff-0553d15257b3cb95586c15820fe4dbe55ce1d4e3bae03f0f37d88fafcc50709b), but I would argue that this PR was a breaking change.
I'm 💯 OK with leaving this be as the fix was very minor on our end, but figured I'd flag the issue in case others run into it.
Changing the behavior of collapsing the white space has broken hundreds of tests. We'll have to stick with 2.2.0.
@rafaelfranca should some documentation or changelog be created to let users know that the update is a breaking change and that the :html equality operator must be used to match all whitespace?