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

WRP-12378: Fixed Scroller and VirtualList to focus Spottable container items properly via page up/down #1437

Merged
merged 2 commits into from
Apr 10, 2023

Conversation

0x64
Copy link
Member

@0x64 0x64 commented Apr 10, 2023

Checklist

  • I have read and understand the contribution guide
  • A CHANGELOG entry is included
  • At least one test case is included for this feature or bug fix
  • Documentation was added or is not needed
  • This is an API breaking change

Issue Resolved / Feature Added

When a page up/down key is pressed on items in a scroller or a list, an expected behavior is to scroll a page and focus an item at the same visual position normally.

And, if page up is pressed at the first page or page down is pressed at the last page, an expected behavior is to scroll to the edge then focus the first (for page up) or the last (for page down) item.

The reported issue is that, the focus disappears on page down key at the last page when the last item is spotlight container that has a spottable node not attached to the bottom of the item.

Resolution

useScroll has a logic to find an item to focus and it was working well for items that are not Spotlight containers as it searches candidates from the last active Spotlight container. To fix this, I changed the starting node from the last active Spotlight container to a scroll container.

Additional Considerations

Links

WRP-12378
#1434

Comments

Enact-DCO-1.0-Signed-off-by: Seungcheon Baek (sc.baek@lge.com)

… page up at the first page and page down at the last page

Enact-DCO-1.0-Signed-off-by: Seungcheon Baek (sc.baek@lge.com)
Enact-DCO-1.0-Signed-off-by: Seungcheon Baek (sc.baek@lge.com)
@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (54c0d8b) 57.35% compared to head (8197fe5) 57.35%.

❗ Current head 8197fe5 differs from pull request most recent head f756ab2. Consider uploading reports for the commit f756ab2 to get more accurate results

Additional details and impacted files
@@                  Coverage Diff                   @@
##           release/2.5.x.develop    #1437   +/-   ##
======================================================
  Coverage                  57.35%   57.35%           
======================================================
  Files                        136      136           
  Lines                       6388     6388           
  Branches                    1883     1883           
======================================================
  Hits                        3664     3664           
  Misses                      2125     2125           
  Partials                     599      599           
Impacted Files Coverage Δ
useScroll/useScroll.js 73.33% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@juwonjeong juwonjeong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@juwonjeong juwonjeong merged commit b67fff0 into release/2.5.x.develop Apr 10, 2023
@juwonjeong juwonjeong deleted the feature/WRP-12378-2.5.x branch April 10, 2023 07:58
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