-
Notifications
You must be signed in to change notification settings - Fork 25
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
fix: properly indent child views of archived tables #909
Conversation
Signed-off-by: Elizabeth Danzberger <[email protected]>
.archived-items .app-navigation-entry__children { | ||
& .app-navigation-entry-wrapper .app-navigation-entry__children { | ||
& .app-navigation-entry { | ||
padding-left: calc(16px * 2); |
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.
Any specific reason to not just use 32px?
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.
I figured it would be clearer to use that since I did not have access to the $icon-size variable which is defined in the Vue library and set at 16px, to make it clearer that that's what I was referencing. The $icon-size variable seems to define the left-padding for the actual view navigation item in the underlying vue component. But yeah this can be changed
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.
All right, would have been fine either way, in that case a code comment might have made sense explaining the calculation, but fine to keep the 32px as well 👍
Signed-off-by: Elizabeth Danzberger <[email protected]>
@elzody could you add before/after screenshot so a design review is easier? :) Thanks! |
No problem, added it now @jancborchardt |
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.
Looks good design-wise! Regarding code rather than putting a set number, might be better to use a multiple of the default-grid-baseline variable? (Which is 4px, so 8* that in this case.)
I'd get this in for now. The offset depends on the icon size which is currently also hard coded in the vue library |
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! |
Fix for issue #892
Added some CSS which will further indent views which are nested in an archived table to make the hierarchy more visible. If there's a better way, definitely let me know :)
Screenshots
Previous
Suggested