Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

add ContentAnchor component #2582

Merged
merged 82 commits into from
Jan 30, 2020
Merged

add ContentAnchor component #2582

merged 82 commits into from
Jan 30, 2020

Conversation

jroebu14
Copy link
Contributor

@jroebu14 jroebu14 commented Nov 7, 2019

Resolves #2279

Overall change: Adds a component that can wrap content such as ads to detect and mitigate content shifting. For more info refer to the README.md


ezgif com-video-to-gif

Above is the Storybook story. To test and showcase the component we have here content that loads a new, different sized image every few seconds. The green shows where the browser repainted sections of the screen as a result of the image changing and causing content reflows. You can see from this example that the content within the wrapper will never resize when in view and hide any overflow of the content within. When it is not in view the wrapper and it's content is allowed to resize and the scroll position is adjusted so the user will not experience any unexpected jumps.


Code changes:

NB

  • This component needs a better name! Suggestion welcome.
  • Ideas for more test cases inside of Storybook are welcome.
  • This is the first time I've used dynamic imports and I haven't seen it used in Simorgh or Psammead so maybe we need to have a discussion about it's use.
  • Needs an a11y swarm
  • IntersectionObserver is polyfilled whereas ResizeObserver is ponyfilled. There's nothing strategic about this, it's just the way these modules were designed. One view is that ponyfills should be preferred over polyfills. I'm not totally convinced but if anyone feels strongly about it I could search for an IntersectionObserver ponyfill
  • Writing good unit tests for this component was a challenge. Open to ideas to make them better. I don't think this can be e2e tested until it is implemented in Simorgh(?).
  • Unit tests are minimised in the file changed tab. Make sure to expand and have a look.
  • Happy to set up a swarm review for this if people feel we need it.

  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

Testing notes
Needs tested in all browsers. Can be tested in Storybook.

@greenc05 greenc05 added the a11y-swarm An a11y swarm (clarify dev or full team in the desc) needs to be carried out before moving to test label Nov 12, 2019
@jroebu14 jroebu14 marked this pull request as ready for review November 18, 2019 09:50
@amywalkerdev amywalkerdev added the ws-home Tasks for the WS Home Team label Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y-swarm An a11y swarm (clarify dev or full team in the desc) needs to be carried out before moving to test ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic component wrapper to avoid content reflow in viewport
8 participants