fix(scroll): scroll direction inconsistency #1629
Merged
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.
Pull request template
After the Vue3 upgrade the
useScroll
composable created a inconsistency in the scroll direction prop in theuseScroll
instance and the scroll direction prop in the Vuex store.Explanation:
When scrolling, the data in
useScroll
is updated via thestoreScrollData
method, which updates values oncurrentPosition
andclientHeight
. For the first scroll event, the behavior is described below:In Vue2 the running order was:
clientHeight
watch, which blocks updates on scroll direction becauseisClientHeightChanging
is set totrue
.currentPosition
watch, skipped due toisClientHeightChanging
= true.But in Vue3 the running order is:
currentPosition
watch, because in the running order ofstoreScrollData
currentPosition is mutated first. ThenisClientHeightChanging
is false, so it mutatesscrollDirection
clientHeight
watch, which blocks updates on scroll direction becauseisClientHeightChanging
is set totrue
.scrollDirection
watch, skipped due toisClientHeightChanging
= true. So the event is not emitted and the store keeps the original value not the current one.Consequences:
In the x-archetype, it is used the
useHasScrollPastThreshold
composable which relies on the scroll direction of the store. It is used in this case to collapse the subheader of the desktop and mobile layouts. Those subheaders were not hidden due to this bug.How it is fixed:
Changing the running order of the
storeScrollData
mutating first theclientHeight
, it is ensured that the data is consistent between the composable and the store.Before:
Screen.Recording.2024-10-01.at.17.43.42.mov
After:
Screen.Recording.2024-10-01.at.17.41.49.mov
Motivation and context
Type of change
What is the destination branch of this PR?
Main
How has this been tested?
Tests performed according to testing guidelines:
Checklist: