Repository navigation
Show a top gradient on the crossword clue list and update gradients while scrolling - #17025
Open
shrinivas-sn wants to merge 1 commit into
Open
shrinivas-sn wants to merge 1 commit into
shrinivas-sn wants to merge 1 commit into
Conversation
…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
This file contains hidden or 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
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.
What does this change?
leftColresets.Layoutinto a smalluseScrollGradientshook insrc/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 thescrollandresizelisteners. 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 asetStatecall sets the value it already has.The top gradient has
pointer-events: noneso it does not block clicks on the first clues.How has this change been tested?
src/lib/useScrollGradients.test.tscover: 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.pnpm test: all suites pass except thecdk/lib/renderingStack.test.tssnapshots on my machine, wheregu:repocomes from my fork'soriginURL. That suite passes withoriginpointed atguardian/dotcom-rendering.Screenshots
None yet, see above.