Skip to content

Show a top gradient on the crossword clue list and update gradients while scrolling - #17025

Open
shrinivas-sn wants to merge 1 commit into
guardian:mainfrom
shrinivas-sn:fix/crossword-clue-list-gradient
Open

shrinivas-sn wants to merge 1 commit into
guardian:mainfrom
shrinivas-sn:fix/crossword-clue-list-gradient

Conversation

@shrinivas-sn

Copy link
Copy Markdown

What does this change?

  • Shows a gradient at the top of the crossword clue list when it has been scrolled away from the start, to match the existing one at the bottom. It uses the same print and leftCol resets.
  • Updates both gradients on every scroll and resize event instead of after a 100ms debounce, so they change while still scrolling.
  • Moves the scroll and resize handling out of Layout into a small useScrollGradients hook in src/lib. It registers one handler for the life of the component and removes that same handler on unmount.

Closes #13543

Why?

The clue list only hinted that it could be scrolled down. Once you scroll, nothing shows there is more above.

The old effect also called libDebounce(...) again when removing the scroll and resize listeners. That creates a different function each time, so the listeners that were added were never removed. A single stable handler fixes that. There is no need to debounce: the state is two booleans, and React does not re-render when a setState call sets the value it already has.

The top gradient has pointer-events: none so it does not block clicks on the first clues.

How has this change been tested?

  • New unit tests in src/lib/useScrollGradients.test.ts cover: bottom gradient only at the start, none when the list does not scroll, both gradients updating during a scroll with fake timers and no timers advanced, a few pixels from either end being ignored, resize updating the state, and one handler being added and the same one removed on unmount. With the old debounced add/remove put back into the hook, 4 of the 6 tests fail.
  • pnpm tsc, eslint and prettier on the changed files, and stylelint on the component, pass.
  • Full pnpm test: all suites pass except the cdk/lib/renderingStack.test.ts snapshots on my machine, where gu:repo comes from my fork's origin URL. That suite passes with origin pointed at guardian/dotcom-rendering.
  • Not done: I have not run this in a browser, and Chromatic and Playwright have not been run. No existing story covers the crossword island, so I would be glad for a reviewer to check how the top gradient looks on the real page.

Screenshots

None yet, see above.

…hile scrolling

The clue list only had a gradient at the bottom, so nothing showed that
it could also be scrolled back up. Add the matching gradient at the top,
with the same print and left column resets.

Both gradients now update on every scroll and resize event instead of
after a 100ms debounce. The old effect also created a new debounced
function when removing the scroll and resize listeners, so the original
listeners were never removed. The logic moves into a useScrollGradients
hook that registers one handler and removes that same handler on unmount.

Closes guardian#13543

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Apply gradient to top and bottom of clue list when scrollable

1 participant