-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Implement dense layout option #15627
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
base: master
Are you sure you want to change the base?
Conversation
Closes nextcloud#15376 Signed-off-by: Philipp Niedermayer <[email protected]>
1b100b6
to
561d986
Compare
Signed-off-by: Philipp Niedermayer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me as a default instead. I'd be very much against this being an option, especially one buried so far in the menu. We recently increased the density of the list view on the web and that went well as far as I can tell.
However, hiding the share button should be discussed separately.
Unless someone else has objections, I'm in favor of this being the default and not configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eltos thanks for that nice improvement. |
Can the option remain to allow hiding the share button (while spacings are always reduced as you suggest)? Or no option just dense layout with share button? Personally, I find the share button quite useless, as the three dot menu contains an easily accessible share menu with more features (local share via other app & various nextcloud share options). The extra button is mainly taking space and causes the filename to be shortened. On PC there is plenty more space, but IMHO mobile UI should account for the fact that less space is available. This is of course my personal opinion. Your decision. Let me know and I can update the PR. |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
I do agree with you from this point of view, but I'm not sure what the full context for its inclusion is. It does certainly make the feature more discoverable, especially to users of the web interface. For this reason, I suggest splitting the PR out into two. Leave this for just the dense layout and create one removing the share button altogether. Then this one can be merged and we can continue the discussion about the share button there. cc @jancborchardt if you want to discuss the share icon later. |
Signed-off-by: Philipp Niedermayer <[email protected]>
cfc5c2a
to
96ad1c9
Compare
I have updated the PR accordingly and created #15714 for the share button topic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good from my side then. :) (not tested)
Adds an option in settings toincrease the layout density.Closes #15376
EDIT: That PR was updated to have dense layout by default, but not remove the share button. Note that the screenshot is still the initial one where also the share button was removed.