Refactored highlight behavior: Preserved original content structure + handle "BODY" selector #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jespernpedersen
changed the title
jespernpedersen
changed the title
jespernpedersen
requested review from
Flavio-Almeida,
Leiyiliro and
mostergaard
Flavio-Almeida
approved these changes
Aug 23, 2023
Leiyiliro
approved these changes
Aug 24, 2023
Leiyiliro
reviewed
Aug 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notice!
This PR is not breaking any tests. Our WPCS test pipeline is currently broken, as we also tested this on the master branch and it was also just today started to fail on that. I have reached out to Frederik from infra to see if it's an issue with GA runners on their end, but this PR can still be reviewed regardless of the broken test pipeline.
Problem
For some highlights, we saw that the html structure had been permanently altered (a page fresh is needed to undo the structure changes) even after navigating away from the individual highlight. It would remove text node items such as
<em><a>etc.After adding new highlightable issues, we discovered that some of them targeted "BODY" tag, in which caused an anomaly with highlighting and caused the page to refresh.
Before (Problem) - Video:
https://github.com/Siteimprove/CMS-plugin-Wordpress/assets/25395486/5f13e37e-a653-4775-b8d9-b866d0560252
Solution
si-full-highlightthat can handle the case of "BODY" tag, by highlighting the entire pageThis ensures that previous highlights are removed and the structure remains intact before applying new highlights and that we can handle the scenario of full page highlights.
After (Solution) - Video:
https://github.com/Siteimprove/CMS-plugin-Wordpress/assets/25395486/82e9a4bb-5b5b-4599-b987-3f067d56a953
Full Page Highlight - Video:
https://github.com/Siteimprove/CMS-plugin-Wordpress/assets/25395486/4aa1e97d-225e-4d39-a205-b6e9f615d3ec