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

fix(contactsmenu): adjust padding to new design #50532

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Jan 29, 2025

  • Resolves: #

Summary

Padding and white space inside the contacts menu has not yet been adjusted to the new clickable area of 34 px.

Before After
image image

Checklist

@st3iny st3iny added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: contacts menu labels Jan 29, 2025
@st3iny st3iny added this to the Nextcloud 32 milestone Jan 29, 2025
@st3iny st3iny self-assigned this Jan 29, 2025
@st3iny
Copy link
Member Author

st3iny commented Jan 29, 2025

/backport to stable31

@st3iny
Copy link
Member Author

st3iny commented Jan 29, 2025

/backport to stable30

@st3iny st3iny force-pushed the fix/contactsmenu/padding branch from 958fb8e to d40031d Compare January 29, 2025 12:40
@ChristophWurst
Copy link
Member

Why does this even need custom styling? Could it just use the default ncaction styles?

@st3iny
Copy link
Member Author

st3iny commented Jan 29, 2025

Why does this even need custom styling? Could it just use the default ncaction styles?

I wish. The problem is that contact menu actions may inject custom image URLs as their icons which cannot be used with our NcAction* components. The components only accept inline SVGs, material design icons or legacy icon classes.

That is why we need custom styles here.

@@ -114,13 +113,9 @@ export default {
&__icon {
width: 20px;
height: 20px;
padding: 12px;
padding: 7px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
padding: 7px;
padding: calc((var(--default-clickable-area) - 20px) / 2);

Copy link
Contributor

Choose a reason for hiding this comment

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

Independent from other styles, only magic number is the icon size we set above.

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Thats not all that changed, the NcAvatar above still uses the 44px. I think the current design is the default, so removing size should work (but not tested).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗️ In progress
Development

Successfully merging this pull request may close these issues.

3 participants