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

feat(compact): follow-up #14118

Merged
merged 4 commits into from
Jan 14, 2025
Merged

feat(compact): follow-up #14118

merged 4 commits into from
Jan 14, 2025

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Jan 12, 2025

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
image image

🚧 Tasks

  • Better solution for maintaining the ellipsis?

🏁 Checklist

@Antreesy
Copy link
Contributor

To not lose this discussion in merged PR:

Inner content has a 4px gap (if there is any unread message, counter bubble is fixed on 22px+2px margin)
image

IMO feels like we can increase icon size by that amount (cc @nimishavijay)

20 24
image image

src/components/LeftSidebar/LeftSidebar.vue Outdated Show resolved Hide resolved
@nimishavijay
Copy link
Member

Agreed, the bigger avatar size looks better!

@DorraJaouad DorraJaouad force-pushed the fix/11697/compact-follow-up branch from 83f9b7b to 7a2e965 Compare January 14, 2025 12:37
@DorraJaouad DorraJaouad force-pushed the fix/11697/compact-follow-up branch from 7a2e965 to 09e62e1 Compare January 14, 2025 12:43
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Still good)

--list-item-height: calc(var(--clickable-area-small, 24px) + 4px) !important;
}

:deep(.list-item--compact .button-vue--size-normal) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also remove box-shadow from the button?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's problematic even in normal view?

Copy link
Contributor

Choose a reason for hiding this comment

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

well, yeah.. that out of scope of this PR

@DorraJaouad DorraJaouad merged commit 790cbb0 into main Jan 14, 2025
50 checks passed
@DorraJaouad DorraJaouad deleted the fix/11697/compact-follow-up branch January 14, 2025 16:42
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.

3 participants