8000 fix(label): update label min-width to match height by sg00dwin · Pull Request #7217 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(label): update label min-width to match height#7217

Merged
mcoker merged 1 commit intopatternfly:mainfrom
sg00dwin:7215-label-min-width
Dec 3, 2024
Merged

fix(label): update label min-width to match height#7217
mcoker merged 1 commit intopatternfly:mainfrom
sg00dwin:7215-label-min-width

Conversation

@sg00dwin
Copy link
Contributor
@sg00dwin sg00dwin commented Nov 14, 2024

fixes #7215
Set label min-width to equal height

@patternfly-build
Copy link
Collaborator
patternfly-build commented Nov 14, 2024

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

This is fine, but I do wonder if we really ever want them to be narrower than they are tall? (ie a circle?) Maybe something to ask design and circle back on 😆

@sg00dwin
Copy link
Contributor Author
sg00dwin commented Nov 19, 2024

This is fine, but I do wonder if we really ever want them to be narrower than they are tall? (ie a circle?) Maybe something to ask design and circle back on 😆

I reached out to Andrew and Lucia and got the thumbs up to maintain a min-width so that a label will not shrink less than it's height and maintain a circle 😁

I will update this pr to handle these changes.

@sg00dwin sg00dwin force-pushed the 7215-label-min-width branch from ad1ae18 to 8d1039d Compare November 19, 2024 22:45
@sg00dwin sg00dwin changed the title fix(label): remove label min-width fix(label): update label min-width to match height Nov 19, 2024
@sg00dwin sg00dwin force-pushed the 7215-label-min-width branch from 8d1039d to 32b228d Compare November 19, 2024 22:47
--#{$label}--m-compact--PaddingInlineStart: var(--pf-t--global--spacer--sm);
--#{$label}--m-compact--FontSize: var(--pf-t--global--font--size--body--sm);
--#{$label}--m-compact--m-editable--TextUnderlineOffset: #{pf-size-prem(1px)};
--#{$label}--m-compact--MinWidth: calc(var(--#{$label}--m-compact--FontSize) * var(--pf-t--global--font--line-height--body));
Copy link
Member

Choose a reason for hiding this comment

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

Should compact use the same formula, adding on the block paddings just in case someone adjusts those variables to something other than zero?

@sg00dwin sg00dwin force-pushed the 7215-label-min-width branch from 32b228d to 433fe6c Compare November 21, 2024 19:20
@sg00dwin sg00dwin force-pushed the 7215-label-min-width branch from 433fe6c to e2f4467 Compare November 21, 2024 19:21
Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Super! 🏅

Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LPTM!

@mcoker mcoker merged commit 19ce976 into patternfly:main Dec 3, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.1.0-prerelease.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Bug - Label - issue with min-width and short labels

4 participants

0