Skip to content

feat(label): add support for overflow - #3339

Merged
mcoker merged 3 commits into
patternfly:masterfrom
christiemolloy:issue-3259
Jul 29, 2020
Merged

mcoker merged 3 commits into
patternfly:masterfrom
christiemolloy:issue-3259

Conversation

@christiemolloy

Copy link
Copy Markdown
Member

closes #3259

@patternfly-build

patternfly-build commented Jul 28, 2020 •

Copy link
Copy Markdown
Collaborator

Preview: https://patternfly-pr-3339.surge.sh

CSS Size Report
NameCurrentPreviousDiff %
components/Label/label.css16.4 kB16.2 kB1.09
patternfly-no-reset.css742.4 kB742.2 kB0.02
patternfly.css744.3 kB744.1 kB0.02
patternfly.min.css655.7 kB655.5 kB0.02

A11y report: https://patternfly-pr-3339-coverage.surge.sh

@mcoker

mcoker commented Jul 28, 2020

Copy link
Copy Markdown
Contributor

@mcarrano @mceledonia would you consider this change visually breaking? Wondering if this should be opt-in, or just the new default for labels.

@mcarrano

Copy link
Copy Markdown
Member

I don't consider it visually breaking. Why wouldn't you want this? What do you think @mceledonia ? Also, isn't the Label component still beta?

@mcoker

mcoker commented Jul 28, 2020

Copy link
Copy Markdown
Contributor

@mcarrano a potential implication is if there are labels currently that exceed the width, the content that is currently visible will not be without having to hover the label to see a tooltip, and that could be unwanted. I think it's fine, since it sounds like it was an oversight not to have it in the first place, so it's more of a bug fix. I just wanted to clarify since the way we make the update in react could depend on wither a prop enables it or not.

@mcarrano

Copy link
Copy Markdown
Member

@christiemolloy @mcoker I'm kind of torn on this one. I guess there could be cases where you would not want the label to truncate. Let me get some other design opinions. @mceledonia @maryshak1996 @kybaker @gdoyle1 what do you think? In your usage of labels, are there cases where you might not want a long label name to truncate?

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

@mattnolting mattnolting left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perf! 🎉

@mcoker
mcoker merged commit f33aaab into patternfly:master Jul 29, 2020
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.

5 participants