CSS: adjust media query breakpoint to improve tablet readability

[nachoparker]

see nextcloud/news#467

<= 938	> 938

[jancborchardt]

What do you think @nextcloud/designers?

[skjnldsv]

Damn, I had a pending branch for that :D
Thanks a lot for taking care of that @nachoparker
There are some javascript to edit as well!

see 1a36280

[nachoparker]

@skjnldsv thanks for pointing that out. I tested briefly with the main default apps and I didn't see that we needed any more changes, but since you have made more progress on this, should we merge your changes here? or do you have a standing PR?

In that case we can close this as duplicate

[skjnldsv]

@nachoparker I did not opened the pr yet :)
Please check my commit and adapt this pr accordingly, or I can also open one based on my branch 😉

[nachoparker]

(sorry I deleted my remote branch accidentally)

@skjnldsv I looked at your changes and tweaked them a bit. The only big change is in apps/files/css/mobile.css, where I feel that it is better to simplify the media query. PTAL.

[skjnldsv]

Another one:

server/apps/files_sharing/css/public.scss

Line 198 in c91ba00

@media only screen and (min-width: 769px) {

:)

[skjnldsv]

@skjnldsv I looked at your changes and tweaked them a bit. The only big change is in apps/files/css/mobile.css, where I feel that it is better to simplify the media query. PTAL.

Ah, I just saw that.
We had a three points break because of the sidebar hiding.
If you take a look at the current state, we want to make sure the file list gets at least 688px width (excluding navigation). Maybe we could keep displaying some informations still when under 1024? I feel it's very empty like that :)

[skjnldsv]

🎺

[nachoparker]

let me get you an animation, it will be easier to explain

[nachoparker]

Ok, this is with the rule

@media only screen and (max-width: 1024px) and (min-width: 769px), only screen and (max-width: 688px) {

, and this is with

@media only screen and (max-width: $breakpoint-mobile) {

If we want to keep three cut points, there needs to be more adjustment to be made. I like it simple but that's just my preference.

[skjnldsv]

@jancborchardt :)

[jancborchardt]

@nachoparker looking at the gifs:

• The first one looks good, but when it’s narrower without the left navigation it only shows the right details inbetween, and not when wider? This version would be good if that’s fixed.
• The second one has a looong period of where it doesn’t show the left navigation but also does not show any details on the right. This is a bit strange → see what I said in point 1 :)

[nachoparker]

Thanks @jancborchardt the first one is what you already have, only substituting 938px -> 1024px, that's what I mean that it needs some more tweaking because it doesn't look good (and I proposed number 2).

Let's see if we can find a sweet spot

[nachoparker]

Ok, so if we want three breakpoints I think this is exactly what we are after -> f902ee7

[skjnldsv]

938 was basically the 688+250 (nav width)
We might be able to find the use of the mobile breakpoint somewhere here :)

[skjnldsv]

I think the idea behind it is that we found that the min width the filelist should have was 688px, (with full view, modified, size, actions.
..) So those breakpoints were there for that.
@jancborchardt ?

[jancborchardt]

@nachoparker nice, that last gif you posted looks perfect to me! 👍 What do you think @nextcloud/designers?

[skjnldsv]

I pushed a commit to use the $navigation-width variable.
wrong gif uploaded, see further comments

[skjnldsv]

rebased and ready to go

[jancborchardt]

@skjnldsv hmm, the gif by @nachoparker looks slightly better than the last one you posted, because it has 1 less breaking step:

1. Show navigation and file details on right
2. Hide navigation, still show file details
3. Hide navigation and file details (mobile view)

You have a strange step between 1 and 2 with "Show navigation but hide file details", which doesn’t seem necessary. The content should always take precedence over the navigation. That is, before any content is hidden, the navigation should be hidden before, as done in @nachoparker’s gif.

What do you think?

[skjnldsv]

What do you think?

Sure :)
Then let's adjust the min width for the file table.
I just used what we were supposed to use before (original calculation).

But looking at the first gif, we don't need two breakpoint then. Only one in 688px.

EDIT: ah no, I took a gif on the wrong branch ^^
This is what it really looks like:

[jancborchardt]

Looking very good now! 👍

(We should also look at how it works with the right sidebar, but that’s a follow-up. :)

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

[go2sh]

@skjnldsv Just a minor nitpick. You commited some git merge/rebase LOCAL REMOTE files to the repo. ;-) See main.js.map

[skjnldsv]

@go2sh what do you mean?

Edit ah damn, I see it now. Thanks!

[nachoparker]

Thanks gents!