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(Avatar): add 2xs Avatar size #1721

Merged
merged 17 commits into from
Sep 13, 2024
Merged

feat(Avatar): add 2xs Avatar size #1721

merged 17 commits into from
Sep 13, 2024

Conversation

PahaN47
Copy link
Contributor

@PahaN47 PahaN47 commented Jul 26, 2024

No description provided.

@PahaN47 PahaN47 requested a review from DakEnviy as a code owner July 26, 2024 14:43
@PahaN47 PahaN47 linked an issue Jul 26, 2024 that may be closed by this pull request
@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@gravity-ui-bot
Copy link
Contributor

Visual Tests Report is ready.

@amje
Copy link
Contributor

amje commented Jul 26, 2024

@PahaN47 Please ensure new size is correctly displayed for Avatar related components: AvatarStack, User, UserLabel

@DakEnviy
Copy link
Contributor

Let's add 2xs size for User component. Also, I believe that 12px font-size would be better in User component for sizes xs and 2xs

#{$block}__avatar + #{$block}__info {
margin-inline-start: 6px;
}

#{$block}__info {
@include user-text-small();
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not correct to use code-inline variant for that. It should inherit font from parent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we shouldn't allow 2xs in User and UserLabel components

@PahaN47 PahaN47 requested review from amje and DakEnviy August 20, 2024 09:49
| `--g-user-font-size` | Name and description font size for `size='s'` and larger |
| `--g-user-line-height` | Name and description line height for `size='s'` and larger |
| `--g-user-small-font-size` | Name font size for `size='xs'` and smaller |
| `--g-user-small-line-height` | Name line height for `size='xs'` and smaller |
Copy link
Contributor

Choose a reason for hiding this comment

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

CSS API is designed to ignore all props, so only 2 vars are relevant: --g-user-font-size, --g-user-line-height. There should not be CSS API for props variants

import type {UserSize} from './types';

export const NO_DESCRIPTION_SIZES: UserSize[] = ['xs', '2xs'];
export const DEFAULT_USER_SIZE: UserSize = 'm';
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
export const DEFAULT_USER_SIZE: UserSize = 'm';
export const DEFAULT_SIZE: UserSize = 'm';

@@ -0,0 +1,4 @@
import type {UserSize} from './types';

export const NO_DESCRIPTION_SIZES: UserSize[] = ['xs', '2xs'];
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
export const NO_DESCRIPTION_SIZES: UserSize[] = ['xs', '2xs'];
export const COMPACT_SIZES: UserSize[] = ['xs', '2xs'];

@PahaN47 PahaN47 requested a review from amje September 11, 2024 13:01
@PahaN47 PahaN47 merged commit 1698d51 into main Sep 13, 2024
6 checks passed
@PahaN47 PahaN47 deleted the avatar-2xs branch September 13, 2024 09:43
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.

Add 2xs Avatar size
4 participants