feat(files): responsive design for mobile and narrow screens #37642
Conversation
skjnldsv
requested review from
Pytal,
artonge and
szaimen
and removed request for
a team
|
I guess the multiselect buttons should also collapse? Apart from that looks good to me! :) |
|
Good catch Simon!! 🚀 |
|
Very cool! Looks great in the screenshot, and completely agreed with Simon! :) |
skjnldsv
force-pushed
the
fix/reactivity-files
branch
from
7f3abbe to
f9d9938
Compare
|
Done (ignore the fake favorite action added for this example only) Peek.11-04-2023.15-08.mp4 |
szaimen
left a comment
szaimen
left a comment
There was a problem hiding this comment.
Tested and works :)
(but didnt review the code)
skjnldsv
force-pushed
the
fix/reactivity-files
branch
from
f9d9938 to
5b979e5
Compare
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
skjnldsv
force-pushed
the
fix/reactivity-files
branch
from
5b979e5 to
306bc2a
Compare
| if (this.clientWidth < 480) { | ||
| return 1 | ||
| } | ||
| if (this.clientWidth < 768) { | ||
| return 2 | ||
| } |
There was a problem hiding this comment.
How is this behaving with the sidebar opened ?
Could we use the container width instead of the client's width?
There was a problem hiding this comment.
Also, would it make sense to have the hiding logic inside nc/vue instead of here?
There was a problem hiding this comment.
I used the values files had before.
It's fair to raise the question a bit more widely I think yes :)
When you say sidebar, you mean navigation or sidebar? (because I've seen it being used for both)
- If Navigation, the sidebar is floating on mobile, so it doesn't affect the main view which takes 100%
- If you say Sidebar (right), the Sidebar takes the full width on mobile, but we should definitely change those values if the sidebar is opened (see the video below)
Peek.12-04-2023.17-46.mp4
There was a problem hiding this comment.
I would merge this and improve the sidebar after raising the topic in another PR? :)
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, I meant the right Sidebar. I am asking for the number of displayed actions. The sidebar reduce the width of the container, but the logic is based on the width of the client window, so it might not work as expected.
My advice would be to use a resize observer on the list container instead of the resize event on the window.
https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver
There was a problem hiding this comment.
As you which, the current behavior is still better than nothing ;)
There was a problem hiding this comment.
After looking around, WOW, you're 100% right Louis, we can do better 👍
I will prepare an issue with RFC and screenshots,
First commit is moving css around
The real code to review is in the second 7f3abbe
Peek.07-04-2023.13-31.mp4
@jancborchardt @nimishavijay
Reference #37627 #36534
PS: I'll wait before compiling assets to avoid rebasing