NXT-19990: Fixed long press scroll between multiple VirtualLists

[vJIYEv]

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
• I have run automated testing and it is passed
• Documentation was added or is not needed
• This is an API breaking change

Issue Resolved / Feature Added

When holding down a down key (long press) to move between multiple horizontally-arranged VirtualLists,
focus stopped and did not continue to the last one.

Resolution

In #361, isFirstEntryRepeat was defined to block the scroll when entered a VirtualList from outside with long press.

const isFirstEntryRepeat = repeat && !hasProcessedKeyDownRef.current;
if (isFirstEntryRepeat) {
    ev.preventDefault();
    ev.stopPropagation();
    resetAccelerator();
    return; 
}

Here, keyDown event handler is blocked if isFirstEntryRepeat is true, which means key was pressed before entering the VirtualList and there wasn't a new key press. this makes scroll can newly start in the VirtualList after new key press.

But in case of focus move between VirtualLists, not between Items in a VirtualList, keyDown handler is needed to keep focus moving between the VirtualLists.
The condition of isFirstEntryRepeat should be detect the focus is moving within the VirtualList or not.
So I added isMovingWithinList condition.

Additionally, I added isMovingWithinList contidion in isOutdatedIndex to prevent a possible issue.

Additional Considerations

Links

NXT-19990

Comments

Enact-DCO-1.0-Signed-off-by: Jiye Kim (jiye.kim@lge.com)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.69%. Comparing base (c73111b) to head (eec9537).

Additional details and impacted files

@@                    Coverage Diff                    @@
##           release/1.9.x.develop     #430      +/-   ##
=========================================================
- Coverage                  81.70%   81.69%   -0.02%     
=========================================================
  Files                        153      153              
  Lines                       7342     7335       -7     
  Branches                    2222     2220       -2     
=========================================================
- Hits                        5999     5992       -7     
  Misses                      1037     1037              
  Partials                     306      306              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[daniel-stoian-lgp]

this still returns before the hasProcessedKeyDownRef.current = true assignment at line 253. That means the bug still reproduces whenever a list is entered mid-list instead of at a boundary .

we need to set here
hasProcessedKeyDownRef.current = true;

[vJIYEv]

isFirstEntryRepeat indicates whether the long press when entering this VirtualList is still ongoing,
and hasProcessedKeyDown indicates whether a new keyDown event occurred after entering this VirtualList.

isFirstEntryRepeat was defined to prevent scrolling when a long press starts outside the VirtualList.
It was intentional that the keyDown event was blocked.

So if we set hasProcessedKeyDownRef.current = true; here, when focus moved into the VirtualList, it will be treated as new key press occured, so the scroll will continue.

In Limestone, scroll stops with long press from outside, so I added isFirstEntryRepeat.
But in Sandstone, scroll continues.

We decided to delay to merge until we find which behavior is correct.

[vJIYEv]

To match VirtualList behavior of Enact and Elemental UI, we decided to scroll works with long press from outside.
As isFirstEntryRepeat was introduced to block it, I removed the codes related to isFirstEntryRepeat.

[daniel-stoian-lgp]

please also add a unit test for the fixed scenario

[bongsok]

LGTM

[daniel-stoian-lgp]

how about adding these unit tests as guards for the future?

` test('should keep blocking repeat keydown for the whole long press that entered the VirtualList', () => {
render(

);

	const list = screen.getByRole('list');
	const item0 = list.children.item(0).children.item(0);

	focus(item0);
	expect(currentFocusIndex).toBe(0);

	keyDownRepeat(40)(item0);
	expect(currentFocusIndex).toBe(0);

	keyDownRepeat(40)(item0);
	expect(currentFocusIndex).toBe(0);

	pressDownKey(item0);
	expect(currentFocusIndex).toBe(1);
});

test('should let a held key leave the VirtualList on repeat instead of swallowing it like a first entry', () => {
	const spotlightId = 'useEvent-specs-leave-on-repeat';
	const keyDownSpy = jest.fn();

	document.addEventListener('keydown', keyDownSpy);

	render(
		<VirtualList
			clientSize={clientSize}
			dataSize={dataSize}
			direction="horizontal"
			itemRenderer={renderItem}
			itemSize={itemSize}
			spotlightId={spotlightId}
		/>
	);

	const list = screen.getByRole('list');
	const item0 = list.children.item(0).children.item(0);

	Spotlight.set(spotlightId, {continue5WayHold: true});

	focus(item0);

	keyDownRepeat(40)(item0);
	expect(currentFocusIndex).toBe(0);
	expect(keyDownSpy).toHaveBeenCalled();

	document.removeEventListener('keydown', keyDownSpy);
	Spotlight.set(spotlightId, {continue5WayHold: false});
});`