8000 fix(table): fixed thead checkbox fitting by mattnolting · Pull Request #7134 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(table): fixed thead checkbox fitting#7134

Merged
mcoker merged 3 commits intopatternfly:mainfrom
mattnolting:fix-table-checkbox-7132
Nov 14, 2024
Merged

fix(table): fixed thead checkbox fitting#7134
mcoker merged 3 commits intopatternfly:mainfrom
mattnolting:fix-table-checkbox-7132

Conversation

@mattnolting
Copy link
Collaborator
@mattnolting mattnolting commented Oct 1, 2024

@mattnolting mattnolting requested review from mcoker and srambach October 1, 2024 15:13
@patternfly-build
Copy link
Collaborator
patternfly-build commented Oct 1, 2024

--#{$table}--cell--MinWidth: 0;
--#{$table}--cell--Width: 1%;

max-width: none;
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about using something like fit-content or min-content? I'm not sure it's any different functionally but worries me less 🙃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Works for me! @srambach updated

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like "fit-content" doesn't work in FF, though "none" does

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

@mcoker mcoker requested a review from srambach November 6, 2024 20:12
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.

The checkbox isn't visible at all in FF, though FWIW that was pre-existing. If there are no concerns with using max-width: none, it looks like that works in FF, too.

@mcoker mcoker mentioned this pull request Nov 11, 2024
@mattnolting mattnolting force-pushed the fix-table-checkbox-7132 branch from 3c59dc4 to 59ebbaa Compare November 13, 2024 17:49
@mattnolting
Copy link
Collaborator Author
mattnolting commented Nov 13, 2024

@mcoker updated .#{$table}__check to max-width: none and verified in FF, Safari, and Chrome

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.

🍗

@mcoker
Copy link
Contributor
mcoker commented Nov 13, 2024

BackstopJS Report for yarn backstop:test --filter="table". The widened labels are from another PR (filed an issue to fix here - #7215)

--#{$table}--cell--MinWidth: 0;
--#{$table}--cell--Width: 1%;

max-width: none;
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

@mcoker mcoker merged commit c0cfdb3 into patternfly:main Nov 14, 2024
@patternfly-build
Copy link
Collaborator

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

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.

Checkbox in empty state get cut off

4 participants

0