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

chore(web): remove tree item thumbnail class 'max-h-[500px]' #14901

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MehringTing
Copy link
Contributor

PixPin_2024-12-24_14-51-16

@@ -12,7 +12,7 @@

{#if items.length > 0}
<div
class="w-full grid grid-cols-2 sm:grid-cols-4 lg:grid-cols-6 2xl:grid-cols-8 gap-2 bg-gray-50 dark:bg-immich-dark-gray/50 rounded-2xl border border-gray-100 dark:border-gray-900 max-h-[500px] overflow-auto immich-scrollbar"
class="w-full grid grid-cols-2 sm:grid-cols-4 lg:grid-cols-6 2xl:grid-cols-8 gap-2 bg-gray-50 dark:bg-immich-dark-gray/50 rounded-2xl border border-gray-100 dark:border-gray-900 overflow-auto immich-scrollbar"
Copy link
Contributor

Choose a reason for hiding this comment

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

The breadcrumb navigation bar scrolls out of view due to this change. It would be better if it remained visible at the top at all times.

Since this div no longer overflows, these classes can be removed:

Suggested change
class="w-full grid grid-cols-2 sm:grid-cols-4 lg:grid-cols-6 2xl:grid-cols-8 gap-2 bg-gray-50 dark:bg-immich-dark-gray/50 rounded-2xl border border-gray-100 dark:border-gray-900 overflow-auto immich-scrollbar"
class="w-full grid grid-cols-2 sm:grid-cols-4 lg:grid-cols-6 2xl:grid-cols-8 gap-2 bg-gray-50 dark:bg-immich-dark-gray/50 rounded-2xl border border-gray-100 dark:border-gray-900"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes,It is better to fix it at the top.

@MehringTing MehringTing force-pushed the chore/tree-item-thumbnail-max-height branch 2 times, most recently from b859931 to c1242ab Compare December 26, 2024 06:46
@alextran1502
Copy link
Contributor

We need the max-height to show the assets of the nested tag/folder

@mertalev
Copy link
Contributor

I think it should work like the file browser does. Having two scroll sections is weird and doesn't make good use of space.

@alextran1502
Copy link
Contributor

Not a bad reasoning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants