Prevent cumulative layout shift when rich workspace is loading

[mejo-]

📝 Summary

Instead of only increasing editor height with growing document length, let's stick to a fixed height (25vh) and use that one already while loading the document.

The editor height still increases when it gets focused.

• Fixes: Avoid cumulative layout shifts on Files pages #2834
• Fixes: [Bug]: The Directory List in files is rendered and then moves again after couple of ms.  #2803

🖼️ Screencasts

🏚️ Before	🏡 After
recording1.webm	recording2.webm

🚧 TODO

• Wait for feedback by @nextcloud/designers

🏁 Checklist

• Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
• Sign-off message is added to all commits

[cypress]

2 flaky tests on run #10169 ↗︎

0

147

1

0

2

Details:

Prevent cumulative layout shift when rich workspace is loading
Project: Text	Commit:748d58af57
Status: Passed	Duration: 03:58 💡
Started: Jun 14, 2023 4:25 PM	Ended: Jun 14, 2023 4:29 PM

nodes/Links.spec.js • 1 flaky test

ViewOutputVideo

Test		Artifacts
... > Link website with selection		OutputScreenshots

api/UsersApi.spec.js • 1 flaky test

ViewOutputVideo

Test		Artifacts
The user mention API > fetches users with valid session		OutputScreenshots

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[szaimen]

Thanks a lot for tackling this! 💙 It is actually my number one problem with rich workspaces and why I am usually disabling it.

I tested this and there is no jumping of elements anymore :) but it should not be shown if there is no readme.md file present right?

[marcoambrosini]

Nice improvement!

[juliusknorr]

The initial jumping is trickier as we'd need to know the availability of a rich workspace before the file list is loaded. We'd need a hook in the files app code to have a synchronous pass over of the propfind data to text before rendering.

[mejo-]

The initial jumping is trickier as we'd need to know the availability of a rich workspace before the file list is loaded. We'd need a hook in the files app code to have a synchronous pass over of the propfind data to text before rendering.

Indeed, I just discovered the same. So inside Text the only thing we can do, is reserve a fixed size for the rich workspace regardless of whether the Readme.md exists or not.

How about always limiting the height of non-focused rich workspace to 150px and make the toolbar display: none if not focused? Only on focus, the toolbar takes space and the editor is expanded? I pushed these changes to the PR now. See the following screencasts:

On large screen	On mobile screen
recording2.webm	recording1.webm

[juliusknorr]

I think we should try to find a way to prefetch the information, then we need no layout shift at all:

Some code paths that could be worth to investigate:

Point when the file list is loaded and being setup before rendering, possibly a good time to emit an event with the file list data? https://github.com/nextcloud/server/blob/8de29843497efed3eff05db1f90a15937d0a504c/apps/files/js/filelist.js#L2284

Maybe there is a way to extend the list of properties dynamically which is taken from https://github.com/nextcloud/server/blob/8de29843497efed3eff05db1f90a15937d0a504c/apps/files/js/filelist.js#L2199 when the file list does the PROPFIND.

We do something similar in group folders but I wouldn't reuse the code there due to the complexity and in depth dependency on files internals: https://github.com/nextcloud/groupfolders/blob/master/src/client.js#L141

[juliusknorr]

Pushed a first draft of my suggested approach to #4776

Seems to work fine except for some quirks with the file list headers and dav properties when navigating around.

[mejo-]

Awesome, let's close this PR in favour of #4776 then.