Skip to content

Conversation

@tanyaka
Copy link
Contributor

@tanyaka tanyaka commented Dec 19, 2024

No description provided.

Copy link
Contributor

@fracado fracado left a comment

Choose a reason for hiding this comment

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

There seems to be no need for a --list-row-height variable. Simply changing the --row-height variable seems to work fine.

Adjustment for breadcrumb on mobile is missing.

Also, the positioning of the icon for the table header for "size" needs to be adjusted.

See review branch: tkl/dev/layout...fb/review/layout

Changes in PR IONOS-Productivity/nc-server#78 don't seem to be needed and commits (apart from the submodule update) can be dropped.

@tanyaka
Copy link
Contributor Author

tanyaka commented Jan 6, 2025

There seems to be no need for a --list-row-height variable. Simply changing the --row-height variable seems to work fine.

Adjustment for breadcrumb on mobile is missing.

Also, the positioning of the icon for the table header for "size" needs to be adjusted.

See review branch: tkl/dev/layout...fb/review/layout

Changes in PR IONOS-Productivity/nc-server#78 don't seem to be needed and commits (apart from the submodule update) can be dropped.

@fracado , thank you so much for your feedback and suggestions, I like them very much!
Concerning the "the positioning of the icon for the table header for "size"" according to design template, the "size" row is not aligned with the header, as well as the folder size in the footer. added alignment with f21f96d

@tanyaka
Copy link
Contributor Author

tanyaka commented Jan 6, 2025

Added fix for thumbnail size for grid view: 1e6fc7c: since the update to NC version 30 the thumbnails in grid view were too large. That was already fixed for new FA icons in apps/files, but probably was lost while update-rebase. So fixed it here.

Copy link
Contributor

@fracado fracado left a comment

Choose a reason for hiding this comment

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

Review OK

@tanyaka tanyaka merged commit 0ab75a4 into main Jan 9, 2025
3 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants