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

Remove appmenu limit #5135

Merged
merged 1 commit into from
May 30, 2017
Merged

Remove appmenu limit #5135

merged 1 commit into from
May 30, 2017

Conversation

patschi
Copy link
Member

@patschi patschi commented May 26, 2017

Will remove the appmenu limit as discussed in #5121.

Signed-off-by: Patrik Kernstock <info@pkern.at>
@mention-bot
Copy link

@patschi, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @icewind1991 and @nickvergessen to be potential reviewers.

@MariusBluem
Copy link
Member

Screenshot:
bildschirmfoto 2017-05-27 um 20 05 51

@MariusBluem MariusBluem added 3. to review Waiting for reviews design Design, UI, UX, etc. labels May 27, 2017
@MariusBluem MariusBluem added this to the Nextcloud 13 milestone May 27, 2017
Copy link
Member

@MariusBluem MariusBluem left a comment

Choose a reason for hiding this comment

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

I think thats OK 👍

@MariusBluem
Copy link
Member

Please review @nextcloud/designers

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

LGTM

@jancborchardt
Copy link
Member

Yup, good point, good change. 👍 :) Now that we have the dynamic sizing always it’s good to also use the space dynamically.

@juliusknorr
Copy link
Member

Should we backport that change? @jancborchardt @karlitschek

@karlitschek
Copy link
Member

Backport is fine in general. But I would like to request a change. I think it looks bad and super crowded if every available pixel in the header is used. Can we make sure that we have a bit more whitespace between the most right app icon and the search icon?

@jancborchardt
Copy link
Member

Sounds good @karlitschek. Let’s keep a minimum of 1 icon width space between the rightmost app icon and the search icon.

What do you think @juliushaertl?

@karlitschek
Copy link
Member

I still think this looks super crowded. What's wrong with some whitespace in the UI? Now we have a web application that is completely filled with buttons.
Saying all of that it is the decision of the design team. Whatever you guys decide is fine with me.

@patschi
Copy link
Member Author

patschi commented Jun 2, 2017

When an user installs quite a lot of apps it may look a bit overloaded:
image

IMO using about 1/3 of the space would be okay:
image

I don't know about the possibilities to manage the appmenu behavior through apps, but someday the appmenu could be managed more flexible using an own app: Ordering the icons, setting the procentual space to use, limit app icons count, hiding specific apps or even overwrite specific app icons, showing up icons only for specific users/groups/devices/resolution, different appmenu settings on different pages, etc.

Until then I'd suggest limiting the icons to 1/3 (or 1/2) of the available space in the navigation. For me the space between each app is fine. The situations were so many apps are installed that it may fill up the whole navigation should be quite rare anyway.

@jancborchardt
Copy link
Member

Right, trying it out for longer it is way too much, and especially since you don’t have the labels. But even with labels there would be way too much noise.

@juliushaertl @eppfel what do you think? I would actually reduce it down to the 8 limit we had before again. It was a nice balance between showing the most important apps, and not having too many.

@patschi
Copy link
Member Author

patschi commented Jun 2, 2017

What about the idea to dynamically set the app limit based on actual window size than on a static limit? On bigger resolutions (QHD, 4k, etc) there is much more space available, so we could use something of that additional space aswell. Showing up to 10-12 apps on a 1920x1080 resolution, more at 4K res, for example. Sadly I don't have any comparisons nor experience with that many different resolutions, or how it may look like on QHD or 4K screens - I'm just having 1080p here.

Personally an app limit of 8 would be a bit too few in my opinion, I'd prefer something like 10-12.

@icewind1991
Copy link
Member

It was a nice balance between showing the most important apps, and not having too many.

The problem is that we dont show the most important apps, we show a random set of apps

@eppfel
Copy link
Member

eppfel commented Jun 2, 2017

I'm really for the solution @patschi proposed, as the numerical amount is not the actual issue here, but the available space. Using 1/2 the width on desktop, seems to be a good call, as this will show a lot more than 8, but help with the visual distinction between the left and right navigation.

@jancborchardt
Copy link
Member

Ok, then let's try out that »use half of the width of the header as maximum« approach. @patschi could you open a pull request? :)

@patschi
Copy link
Member Author

patschi commented Jun 4, 2017

PR #5244 for percentual appLimit. I've choosen to use 33% for now.

As comparison on a 1080p width resolution:
33%
image

40%
image

50%
image

Edit after MorrisJobkes comment
Phone with 1080x1920p res
Phone: Portrait
image

Phone: Landscape
image

@MorrisJobke
Copy link
Member

Keep in mind that it should still work on mobile and small screens. I would love to see this as a max-width setting for bigger screens.

@jancborchardt
Copy link
Member

Yeah, on a phone the percentage doesn’t fully work. On narrow widths it should work like before. On bigger screens the 33% rule can work.

@juliusknorr
Copy link
Member

How about using the percentage rule if there is space for more that 8 apps and on smaller screens just show as much as possible with a 2-3x icon-width as space.

@Espina2
Copy link
Contributor

Espina2 commented Jun 7, 2017

Since the first day I am against this approach, and again in just use the icon to identify an app. Like @karlitschek said this interface look chaotic and have too many entries all over the place, actions on the left sidebar on top bar, on the files.

I know this comment doesn't solve anything but just want to know that I 100% against this solution at first hand.

I keep thinking that Nextcloud is no longer a file sharing software but more like a suite of applications. Like the claim on Homepage "Your Secure Workspace".

Why not have instead of files on the first screen something that you can choose what do you want to go. (just thinking out loud).

I don't mean to disrespect the work of anybody here, this is are better than it has before.

@jancborchardt
Copy link
Member

@juliushaertl exactly.

I keep thinking that Nextcloud is no longer a file sharing software but more like a suite of applications. Like the claim on Homepage "Your Secure Workspace".

@Espina2 That's exactly the case, and that was our goal since very early on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.