Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure scroll posotion resets to top when logo clicked #4328

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AndyKilmory
Copy link
Collaborator

What does this change?

This PR fixes a regression introduced through a combination of PRs that removed ambiguity regarding the 'nonFree' parameter in the current search context. Previously when the logo was clicked the search returned to the top of the page i.e. positionTop = 0 - the code was recording the position of the scroll when the logo was clicked but the setting of nonFree to undefined ensured that the contexts did not match and thus the scroll was no reset to its previous location.

The introduction of typescript react contros has made the use of nonFree = undefined problematic as typescript won't accept that value for a boolean so we have had to do introduce some code changes to ensure both scrolling and page navigation work correctly by removing the ambiguity of nonFree = false/undefined - in some areas the code relied on these being equivalent and in others it required them to be treated as different - e.g. resetting scroll position.

The recent PR to fix issues around page to page navigation and providing correct nonFree values to typescript controls removed further ambiguity around the nonFree parameter such that during logo click process the contexts evaluate as equal and thus the scroll position was reset to its previous value rather than being left at page-top.

The removal of the ambiguity around nonFree is required for other navigations to operate correctly with the typescript controls and so this PR introduces a new function for the scroll service to allow user to say 'reset to top' - this function is called during the logo click process and ensure that positionTop is set to 0 and thus when the contexts evaluate to equal the reset scroll position is now top.

How should a reviewer test this change?

Ensure that on logo click the page resets to the top of the image list.

Who should look at this?

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

…ow ambiguity around nonFree has been removed from current search context
@AndyKilmory AndyKilmory requested review from a team as code owners September 16, 2024 10:41
@twrichards
Copy link
Contributor

building for our test environment now, hopefully review and merge this afternoon...

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.

2 participants