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

[full-ci] enable lazy table cells and fix tests #6204

Merged
merged 2 commits into from
Jan 6, 2022
Merged

Conversation

fschade
Copy link
Contributor

@fschade fschade commented Dec 31, 2021

Description

since lazy table cell rendering has landed in ODS we also should use it in web. the feature is enabled by default, but can be disabled via the configuration ("resourceTableLazy": false).

Related Issue

Motivation and Context

as written in linked issue, current views kinda slow and should be snappy instead. This is just one piece of it and wiill need further investigation.

How Has This Been Tested?

  • local installation, unit tests and integration tests.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added

@fschade fschade added the Status:Needs-Review Needs review from a maintainer label Dec 31, 2021
@fschade fschade self-assigned this Dec 31, 2021
@ownclouders
Copy link
Contributor

Results for oC10SharingPublic1 https://drone.owncloud.com/owncloud/web/21471/37/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10Files2 https://drone.owncloud.com/owncloud/web/21471/17/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@kulmann
Copy link
Contributor

kulmann commented Jan 3, 2022

Currently blocked by #6142

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

  • See comment about the option name
  • Please add a line to the docs about the new option

packages/web-runtime/src/store/config.js Outdated Show resolved Hide resolved
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Can't comment on the code quality since it'll rebased anyhow after the icons have been changed, but feels amazing in the UI already!
Would it be an improvement to switch to lazy loading of rows instead of cells from a UX perspective?

@kulmann
Copy link
Contributor

kulmann commented Jan 4, 2022

Would it be an improvement to switch to lazy loading of rows instead of cells from a UX perspective?

Would be an additional improvement, but for now the cells are good. The extra work is better spent in cleaning up the vuex store.

@ownclouders
Copy link
Contributor

Results for oc10SharingIntUsers3 https://drone.owncloud.com/owncloud/web/21555/32/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@fschade fschade changed the title enable lazy table cells and fix tests [full-ci] enable lazy table cells and fix tests Jan 4, 2022
@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/21557/15/1
The following scenarios passed on retry:

  • webUISharingAcceptSharesToRoot/acceptShares.feature:108

@pascalwengerter
Copy link
Contributor

Restarting CI, only a single pipeline failed and that one works fine locally...

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Not sure if we want to have the dockerized configs with or without lazy tables. But that can be decided later on as well. PR is good to go from my POV 👍

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 6, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

80.0% 80.0% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit 216fd44 into master Jan 6, 2022
@delete-merged-branch delete-merged-branch bot deleted the lazy-table-cells branch January 6, 2022 20:58
@lookacat lookacat mentioned this pull request Jan 9, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants