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

WRO-478: POC for item reordering in VirtualList #1293

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

SkylerBaek
Copy link
Member

@SkylerBaek SkylerBaek commented Jul 26, 2022

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

POC for item reordering in VirtualList

Resolution

The former POC implementation of #1189 is totally refined.

  • Changed to wait for press-and-hold input to edit items' order
  • Changed to get a single object-typed editable prop rather than several individual props
  • Added a selected effect for a sample

Additional Considerations

There are so many bugs and missing behaviors.

  • Touch input is not supported yet.
  • UI could be broken when page up/down key is pressed during reordering.
  • UI could be broken or ugly when boundaries are changed.
  • It is unstable when various input devices are used simultaneously.
  • I cannot guarantee that it works great even with a single input device.
  • Performance optimization is required.

Links

WRO-478 (companion enactjs/enact#3076)
WRN-17866 (#1189)

Comments

Enact-DCO-1.0-Signed-off-by: Seungcheon Baek ([email protected])

Enact-DCO-1.0-Signed-off-by: Seungcheon Baek ([email protected])
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #1293 (b303945) into develop (904daf8) will increase coverage by 0.36%.
The diff coverage is 15.38%.

@@             Coverage Diff             @@
##           develop    #1293      +/-   ##
===========================================
+ Coverage    53.74%   54.10%   +0.36%     
===========================================
  Files          137      137              
  Lines         6671     6818     +147     
  Branches      1967     2025      +58     
===========================================
+ Hits          3585     3689     +104     
- Misses        2400     2432      +32     
- Partials       686      697      +11     
Impacted Files Coverage Δ
VirtualList/VirtualList.js 73.33% <ø> (ø)
VirtualList/useThemeVirtualList.js 42.50% <ø> (ø)
VirtualList/useEvent.js 20.19% <13.15%> (-1.49%) ⬇️
useScroll/useScroll.js 73.33% <100.00%> (ø)
FixedPopupPanels/FixedPopupPanels.js 70.96% <0.00%> (-2.37%) ⬇️
PopupTabLayout/PopupTabLayout.js 48.00% <0.00%> (-0.72%) ⬇️
Input/Input.js 97.56% <0.00%> (+1.00%) ⬆️
Input/InputFieldSpotlightDecorator.js 83.87% <0.00%> (+6.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 904daf8...b303945. Read the comment docs.

@seunghoh
Copy link
Member

@0x64 Would you check the Travis error?

@SkylerBaek
Copy link
Member Author

@0x64 Would you check the Travis error?

@seunghoh,
Travis fails since the companion enact branch is not matched. I did not update travis.yml as I thought this branch would be simply a reference for a new feature that is not officially taken in yet.
For maintainers, I will update travis.yml. :)

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