Bugfix: Gridview whitespace autosize #10118

lookacat · 2023-12-05T10:23:38Z

Description

We've fixed a bug that caused the tiles-view to have whitespace
on the right side of the screen which is not optimal for efficiant
space management.
See #10040

Related Issue

Fixes Optimize tiles view #10040

Screenshots (if appropriate):

Looks bit weird cause of the screenrecording but looks smooth in realtime
chrome-capture-2023-11-5.webm

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:

update-docs · 2023-12-05T10:23:43Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

kulmann

Yeah, awesome!!! ❤️

tbsbdr · 2023-12-05T15:07:41Z

love it 🔥
I found 1 thing:
if you have only few items, these are streched over the whole width, ignoring the slider setting 😬

tbsbdr

please see #10118 (comment)

kulmann

Having an empty folder and inserting items one by one feels very weird, since the tile size is shrinking with every new item. I would expect the tile size to be stable. Having only one non-filled line also looks wrong, since the gap sizes are quite huge. I'll attach a series of screenshots to visualize the issue.

I'd expect the tile size like in the last screenshots for all other screenshots as well. Maybe a solution with aria-hidden dummy items, like @janackermann proposed in a call earlier today, is working better. Could you give that a try?

packages/web-pkg/src/components/ViewOptions.vue

lookacat · 2023-12-12T07:46:38Z

@kulmann yes will try to do it that way

packages/web-app-files/src/components/FilesList/ResourceTiles.vue

AlexAndBear · 2023-12-16T12:18:24Z

@lookacat I cleaned up the code a little, I like the way where this is going and it looks already fine, but unfortantly the pr renders too much ghost tiles

AlexAndBear

Check why too many ghost tiles are rendered

CLAassistant · 2023-12-18T09:33:51Z

All committers have signed the CLA.

packages/web-pkg/src/composables/viewMode/useTileSize.ts

packages/web-app-files/tests/unit/components/FilesList/ResourceTiles.spec.ts

worked on the PR myself

packages/web-app-files/src/components/FilesList/ResourceTiles.vue

JammingBen

LGTM overall. We should probably resolve my first remark, but IMO not a blocker since it works for now.

JammingBen · 2023-12-20T08:55:20Z

packages/web-app-files/src/components/FilesList/ResourceTiles.vue

@@ -363,6 +385,50 @@ export default defineComponent({
      context.emit('fileDropped', resource.id)
    }

+    const viewWidth = ref<number>(0)
+    const updateViewWidth = () => {
+      const element = document.getElementById('tiles-view')


Feels quite weird to set the id tiles-view from the outside and then reference it here because it creates an indirect dependency. I think it would be cleaner to set the id here in the component, or better use a template ref.

packages/web-pkg/src/composables/viewMode/useTileSize.ts

sonarcloud · 2023-12-20T09:54:44Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
57.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

* Bugfix: Gridview whitespace autosize * Fix large scale * Fix viewoption sizes * Add changelog * Update unittests * WIP * Solution? * WIP * WIP * Remove dev leftover * cleanup code * Calculate max tiles count per row / still needs to be recalculated when the slider value changes * Implement watch for tile-size slider changes * Rename rowCount * Lint, change var to const, remove unnecessary computed * Hide from screen readers * feat: introduce useTileSize composable and watch that for tile sizes * Fix undefined issues * slim mount function * fix: tile pixels and ghost tile count * test: fix viewOptions unit tests * Fixing ResourceTiles.spec.ts tests (WIP) * Fix ResourceTiles.spec.ts * Remove dev leftover --------- Co-authored-by: Jan Ackermann <[email protected]> Co-authored-by: Benedikt Kulmann <[email protected]>

lookacat marked this pull request as ready for review December 5, 2023 13:25

lookacat requested review from kulmann, AlexAndBear and JammingBen December 5, 2023 13:26

kulmann approved these changes Dec 5, 2023

View reviewed changes

AlexAndBear approved these changes Dec 5, 2023

View reviewed changes

tbsbdr self-requested a review December 5, 2023 16:12

tbsbdr previously requested changes Dec 5, 2023

View reviewed changes

lookacat force-pushed the bugfix-gridview-whitespace-autosize branch from 2232e28 to d0a2db0 Compare December 11, 2023 13:54

kulmann previously requested changes Dec 11, 2023

View reviewed changes

packages/web-pkg/src/components/ViewOptions.vue Outdated Show resolved Hide resolved

lookacat force-pushed the bugfix-gridview-whitespace-autosize branch from 0832303 to fa91632 Compare December 12, 2023 13:10

lookacat requested review from kulmann and tbsbdr December 12, 2023 13:10

AlexAndBear self-requested a review December 14, 2023 13:23

lookacat force-pushed the bugfix-gridview-whitespace-autosize branch from 87c89f1 to 04ee2f0 Compare December 15, 2023 10:32

AlexAndBear reviewed Dec 16, 2023

View reviewed changes

packages/web-app-files/src/components/FilesList/ResourceTiles.vue Outdated Show resolved Hide resolved

AlexAndBear reviewed Dec 16, 2023

View reviewed changes

packages/web-app-files/src/components/FilesList/ResourceTiles.vue Outdated Show resolved Hide resolved

AlexAndBear self-requested a review December 16, 2023 11:44

AlexAndBear force-pushed the bugfix-gridview-whitespace-autosize branch from 04ee2f0 to a0bb25c Compare December 16, 2023 11:51

AlexAndBear reviewed Dec 16, 2023

View reviewed changes

packages/web-app-files/src/components/FilesList/ResourceTiles.vue Outdated Show resolved Hide resolved

AlexAndBear self-requested a review December 16, 2023 12:18

AlexAndBear requested changes Dec 16, 2023

View reviewed changes

AlexAndBear force-pushed the bugfix-gridview-whitespace-autosize branch from b1f14ad to efce7c4 Compare December 16, 2023 12:29

kulmann force-pushed the bugfix-gridview-whitespace-autosize branch from efce7c4 to ea26acb Compare December 18, 2023 09:33

kulmann changed the base branch from master to stable-8.0 December 18, 2023 09:33

owncloud deleted a comment from AlexAndBear Dec 18, 2023

AlexAndBear reviewed Dec 18, 2023

View reviewed changes

packages/web-pkg/src/composables/viewMode/useTileSize.ts Outdated Show resolved Hide resolved

lookacat and others added 7 commits December 18, 2023 14:59

Fix undefined issues

7c652f8

slim mount function

18b8a9d

fix: tile pixels and ghost tile count

5bdb247

test: fix viewOptions unit tests

a9bbe6c

Fixing ResourceTiles.spec.ts tests (WIP)

06f1ad4

Fix ResourceTiles.spec.ts

ed1870f

Remove dev leftover

63eafdd

lookacat requested a review from AlexAndBear December 19, 2023 11:06

lookacat enabled auto-merge (squash) December 19, 2023 11:07

kulmann reviewed Dec 19, 2023

View reviewed changes

packages/web-app-files/tests/unit/components/FilesList/ResourceTiles.spec.ts Outdated Show resolved Hide resolved

Update tileSizePixels in tests

ae3d2b2

kulmann approved these changes Dec 19, 2023

View reviewed changes

docs: add tile size vars to theme docs

3b59e97

AlexAndBear requested changes Dec 20, 2023

View reviewed changes

packages/web-app-files/src/components/FilesList/ResourceTiles.vue Outdated Show resolved Hide resolved

JammingBen approved these changes Dec 20, 2023

View reviewed changes

Address PR issues

a00230c

lookacat requested a review from AlexAndBear December 20, 2023 09:35

Make linter happy

ed4c794

AlexAndBear approved these changes Dec 20, 2023

View reviewed changes

lookacat merged commit 16bb36a into stable-8.0 Dec 20, 2023
3 checks passed

delete-merged-branch bot deleted the bugfix-gridview-whitespace-autosize branch December 20, 2023 13:52

JammingBen mentioned this pull request Dec 20, 2023

Optimize tiles view #10040

Closed

micbar mentioned this pull request Dec 27, 2023

Release 5.0.0 owncloud/ocis#7940

Closed

71 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: Gridview whitespace autosize #10118

Bugfix: Gridview whitespace autosize #10118

lookacat commented Dec 5, 2023 •

edited

Loading

update-docs bot commented Dec 5, 2023

kulmann left a comment

tbsbdr commented Dec 5, 2023

tbsbdr left a comment

kulmann left a comment

lookacat commented Dec 12, 2023

AlexAndBear commented Dec 16, 2023

AlexAndBear left a comment

CLAassistant commented Dec 18, 2023 •

edited

Loading

JammingBen left a comment

JammingBen Dec 20, 2023

sonarcloud bot commented Dec 20, 2023

Bugfix: Gridview whitespace autosize #10118

Bugfix: Gridview whitespace autosize #10118

Conversation

lookacat commented Dec 5, 2023 • edited Loading

Description

Related Issue

Screenshots (if appropriate):

Types of changes

Checklist:

update-docs bot commented Dec 5, 2023

kulmann left a comment

Choose a reason for hiding this comment

tbsbdr commented Dec 5, 2023

tbsbdr left a comment

Choose a reason for hiding this comment

kulmann left a comment

Choose a reason for hiding this comment

lookacat commented Dec 12, 2023

AlexAndBear commented Dec 16, 2023

AlexAndBear left a comment

Choose a reason for hiding this comment

CLAassistant commented Dec 18, 2023 • edited Loading

JammingBen left a comment

Choose a reason for hiding this comment

JammingBen Dec 20, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Dec 20, 2023

Quality Gate passed

lookacat commented Dec 5, 2023 •

edited

Loading

CLAassistant commented Dec 18, 2023 •

edited

Loading