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

Files table virtual scroller #2280

Merged
merged 1 commit into from
Dec 17, 2019
Merged

Files table virtual scroller #2280

merged 1 commit into from
Dec 17, 2019

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Oct 16, 2019

Description

Implement virtual scroller into FileList component and adjust layout in affected lists.

Related Issue

Motivation and Context

Smooth experience when rendering loaded items.

How Has This Been Tested?

  • test environment: Manually
  1. Have 10 000 files in one folder and try to load them
  2. Try creating a new file/folder
  3. Try deleting some file/folder

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Test in ie11
  • Fix layout of table
  • Implement also in shared with list

@LukasHirt LukasHirt self-assigned this Oct 16, 2019
@LukasHirt LukasHirt changed the title Files table virtual scroller [POCFiles table virtual scroller Oct 16, 2019
@LukasHirt LukasHirt changed the title [POCFiles table virtual scroller [POC] Files table virtual scroller Oct 16, 2019
@DeepDiver1975 DeepDiver1975 mentioned this pull request Oct 21, 2019
10 tasks
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@PVince81
Copy link
Contributor

as discussed, see if this brings us closed to fixed table headers

@PVince81
Copy link
Contributor

also discussed: if we go with a generic file list component or list component as in #2077 (comment), we could integrate virtual scrolling there by default

@LukasHirt
Copy link
Collaborator Author

LukasHirt commented Oct 21, 2019

Let's see what tests will say now. If they'll pass I think this PR will be ready for reviews. I still need to test it in IE11 though.

@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@owncloud owncloud deleted a comment from ownclouders Oct 21, 2019
@ownclouders
Copy link
Contributor

💥 Acceptance tests Files failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7169/

20191213-124814-353.png
20191213-124850-558.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7170/

20191213-144743-721.png
20191213-144816-464.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingAcceptShares failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7170/

20191213-144947-604.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests XGAPortrait failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7170/

20191213-145128-034.png
20191213-145202-266.png
20191213-145238-671.png
20191213-145315-607.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests iPhone failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7170/

20191213-145236-275.png
20191213-145328-669.png
20191213-145422-990.png
20191213-145458-150.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Files failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7170/

20191213-144930-532.png
20191213-145008-900.png
20191213-152219-971.png
20191213-152256-065.png
20191213-152331-974.png
20191213-152407-974.png
20191213-152443-396.png
20191213-152520-878.png
20191213-152600-671.png
20191213-152657-410.png
20191213-152738-362.png
20191213-152836-843.png
20191213-152934-008.png
20191213-153011-526.png
20191213-153049-457.png
20191213-153129-931.png
20191213-153230-901.png
20191213-153311-566.png
20191213-153409-985.png
20191213-153509-207.png
20191213-153548-971.png
20191213-153709-424.png
20191213-153749-166.png
20191213-153829-930.png
20191213-153928-748.png
20191213-154010-584.png
20191213-154131-093.png
20191213-154212-454.png
20191213-154254-338.png
20191213-154334-761.png
20191213-154437-871.png
20191213-154539-121.png
20191213-154642-903.png
20191213-154727-329.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingExternal failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7202/

20191216-091002-810.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests XGAPortrait failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7202/

20191216-091205-560.png
20191216-091238-867.png
20191216-091315-079.png
20191216-091350-893.png

@LukasHirt LukasHirt force-pushed the feature/virtual-scroller branch 5 times, most recently from 2fe4748 to 729ee6c Compare December 16, 2019 15:45
@PVince81
Copy link
Contributor

conflict 🙈

Copy link
Member

@skshetry skshetry left a comment

Choose a reason for hiding this comment

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

Nitpicky: There's just too many empty lines/separation of the block on tests. They are all related, so, any reason to separate those?

Copy link
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

jut minor issues

tests/acceptance/features/webUIFiles/fileDetails.feature Outdated Show resolved Hide resolved
tests/acceptance/pageObjects/FilesPageElement/filesList.js Outdated Show resolved Hide resolved
tests/acceptance/pageObjects/FilesPageElement/filesList.js Outdated Show resolved Hide resolved
Fixed sizes of columns

Made responsive

Adjusted layout

Adjusted shared with list

Bring back id for the list

Moved virtual scroller into files app dependencies and added polyfill

Make table header fixed

Use only class selector for file row

Added border and selected background colour

Scroll files list

Fixed file row selector

Scroll with interval

Moved findFile into custom commands and added wait for filesListContainer

Fixed wrong name in private link feature

Fixed wrong item type

Moved row actions into own component and moved after files list

Made functions async and escape double quotes

Count files via counters in footer and fixed scrollToTop function

Check if the item is actually visible and not only rendered and await click on row for private link

Fixed trashbin delete tests

Finishing touches for scroll functions and adjusted sharing functions to await scrolling

Adjusted resharing tests

Adjusted sharing and distance of scrolling

Add position of header to get properly visible items

Re-enable correct step in shared-with pages tests

Bring waitForFileVisible into versions test

Fixed mobile view

Adjusted order of scroll for opening of sidebar and wait for file in shared with pages

Use shareId as default id and get rid of two TODOs

Remove unneccesary logs

Check if list container is visible in scrollToTop, reorder calls for different scrolling and reuse filename attribute

Added isVisible data state

Adjusted selector for trashbin

Changed selector for actions dropdown

Escape double quotes

Adjusted dependencies

Adjusted filter selectors and label after ods update

Fixed order of parameters

Create replaceChar function
@LukasHirt
Copy link
Collaborator Author

Nitpicky: There's just too many empty lines/separation of the block on tests. They are all related, so, any reason to separate those?

I'm separating pieces of code with new lines to make it more readable.

@LukasHirt
Copy link
Collaborator Author

@individual-it Changes implemented.

@LukasHirt
Copy link
Collaborator Author

Opened an issue for virtual scroll in trashbin - #2723

@LukasHirt LukasHirt merged commit e9dced7 into master Dec 17, 2019
@LukasHirt LukasHirt deleted the feature/virtual-scroller branch December 17, 2019 11:58
@individual-it individual-it mentioned this pull request Dec 19, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants