Feat(chip, label): Add wrappers for content and actions to manage styling embedded components #5360

srambach · 2023-01-30T19:45:57Z

This adds wrappers for content (chip only) and the close button (both chip and label) so that the styling of other components (button and badge) is unnecessary or isolated. The new wrappers are spaced using flex with a column gap.

Breaking changes:

Creates pf-c-chip__actions and pf-c-chip__content
Changes --pf-c-chip__c-button--XXX variables to be --pf-c-chip-actions__c-button--XXX
--pf-c-chip__c-badge--MarginLeft is no longer needed and is removed
Creates pf-c-label__actions
Changes --pf-c-label__c-button--XXX variables to be --pf-c-label-actions__c-button--XXX

patternfly-build · 2023-01-30T19:50:49Z

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

A11y report: https://patternfly-pr-5360-a11y.surge.sh

mcoker

LGTM! Just a few nits. I'll also call out that the negative margin on buttons in the actions container will cause adjacent actions to overlap.

Maybe we move those margins to the actions container?

mcoker · 2023-02-07T16:10:18Z

src/patternfly/components/Chip/chip.scss

+  --pf-c-chip-actions__c-button--PaddingTop: var(--pf-global--spacer--xs);
+  --pf-c-chip-actions__c-button--PaddingRight: var(--pf-global--spacer--sm);
+  --pf-c-chip-actions__c-button--PaddingBottom: var(--pf-global--spacer--xs);
+  --pf-c-chip-actions__c-button--PaddingLeft: var(--pf-global--spacer--sm);
+  --pf-c-chip-actions__c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);
+  --pf-c-chip-actions__c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);
+  --pf-c-chip-actions__c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);
+  --pf-c-chip-actions__c-button--FontSize: var(--pf-global--FontSize--xs);


Suggested change

--pf-c-chip-actions__c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-chip-actions__c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-chip-actions__c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-chip-actions__c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-chip-actions__c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);

--pf-c-chip-actions__c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);

--pf-c-chip-actions__c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);

--pf-c-chip-actions__c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-chip-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-chip-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-chip-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-chip-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-chip-actions--c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);

--pf-c-chip-actions--c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);

--pf-c-chip-actions--c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);

--pf-c-chip-actions--c-button--FontSize: var(--pf-global--FontSize--xs);

mcoker · 2023-02-07T17:07:13Z

src/patternfly/components/Chip/chip.scss

+    --pf-c-button--PaddingTop: var(--pf-c-chip-actions__c-button--PaddingTop);
+    --pf-c-button--PaddingRight: var(--pf-c-chip-actions__c-button--PaddingRight);
+    --pf-c-button--PaddingBottom: var(--pf-c-chip-actions__c-button--PaddingBottom);
+    --pf-c-button--PaddingLeft: var(--pf-c-chip-actions__c-button--PaddingLeft);
+    --pf-c-button--FontSize: var(--pf-c-chip-actions__c-button--FontSize);
+
+    margin-top: var(--pf-c-chip-actions__c-button--MarginTop);
+    margin-right: var(--pf-c-chip-actions__c-button--MarginRight);
+    margin-bottom: var(--pf-c-chip-actions__c-button--MarginBottom);


Suggested change

--pf-c-button--PaddingTop: var(--pf-c-chip-actions__c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-chip-actions__c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-chip-actions__c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-chip-actions__c-button--PaddingLeft);

--pf-c-button--FontSize: var(--pf-c-chip-actions__c-button--FontSize);

margin-top: var(--pf-c-chip-actions__c-button--MarginTop);

margin-right: var(--pf-c-chip-actions__c-button--MarginRight);

margin-bottom: var(--pf-c-chip-actions__c-button--MarginBottom);

--pf-c-button--PaddingTop: var(--pf-c-chip-actions--c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-chip-actions--c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-chip-actions--c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-chip-actions--c-button--PaddingLeft);

--pf-c-button--FontSize: var(--pf-c-chip-actions--c-button--FontSize);

margin-top: var(--pf-c-chip-actions--c-button--MarginTop);

margin-right: var(--pf-c-chip-actions--c-button--MarginRight);

margin-bottom: var(--pf-c-chip-actions--c-button--MarginBottom);

mcoker · 2023-02-07T17:11:39Z

src/patternfly/components/Label/examples/Label.md

 | `.pf-c-label__icon` | `<span>` | Initiates a label icon. |
 | `.pf-c-label__text` | `<span>` | Initiates label text. |
 | `.pf-c-label__editable-text` | `<button>`, `<input>` | Initiates editable label text. See the [editable](#editable) example for details about handling behavior in Javascript.|
+| `.pf-c-label__actions` | `<div>` | Creates a wrapper for label actions. **Required for removeable labels** |


mcoker · 2023-02-07T17:13:33Z

src/patternfly/components/Label/label.scss

+  --pf-c-label-actions__c-button--FontSize: var(--pf-global--FontSize--xs);
+  --pf-c-label-actions__c-button--MarginTop: calc(var(--pf-global--spacer--form-element) * -1);
+  --pf-c-label-actions__c-button--MarginRight: calc(var(--pf-global--spacer--form-element) * -1);
+  --pf-c-label-actions__c-button--MarginBottom: calc(var(--pf-global--spacer--form-element) * -1);
+  --pf-c-label-actions__c-button--PaddingTop: var(--pf-global--spacer--xs);
+  --pf-c-label-actions__c-button--PaddingRight: var(--pf-global--spacer--sm);
+  --pf-c-label-actions__c-button--PaddingBottom: var(--pf-global--spacer--xs);
+  --pf-c-label-actions__c-button--PaddingLeft: var(--pf-global--spacer--sm);


Suggested change

--pf-c-label-actions__c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-label-actions__c-button--MarginTop: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions__c-button--MarginRight: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions__c-button--MarginBottom: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions__c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-label-actions__c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-label-actions__c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-label-actions__c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-label-actions--c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-label-actions--c-button--MarginTop: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions--c-button--MarginRight: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions--c-button--MarginBottom: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-label-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-label-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-label-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

mcoker · 2023-02-07T17:15:27Z

src/patternfly/components/Chip/chip.scss

-    margin-top: var(--pf-c-chip__c-button--MarginTop);
-    margin-right: var(--pf-c-chip__c-button--MarginRight);
-    margin-bottom: var(--pf-c-chip__c-button--MarginBottom);
+  .pf-c-chip__content {


can this be un-nested from the .pf-c-chip parent container? It's creating extra specificity.

mcoker · 2023-02-07T17:15:32Z

src/patternfly/components/Chip/chip.scss

+    font-size: var(--pf-c-chip__content--FontSize);
+  }
+
+  .pf-c-chip__actions {


can this be un-nested from the .pf-c-chip parent container? It's creating extra specificity.

mcoker · 2023-02-07T17:16:10Z

src/patternfly/components/Chip/chip.scss

-  .pf-c-badge {
-    margin-left: var(--pf-c-chip__c-badge--MarginLeft);
+  // Actions Button
+  .pf-c-chip__actions .pf-c-button {


Can you nest this as .pf-c-button { ... } under the existing .pf-c-chip__actions selector?

mcoker · 2023-02-07T17:17:19Z

src/patternfly/components/Label/label.scss

+    --pf-c-button--FontSize: var(--pf-c-label-actions__c-button--FontSize);
+    --pf-c-button--PaddingTop: var(--pf-c-label-actions__c-button--PaddingTop);
+    --pf-c-button--PaddingRight: var(--pf-c-label-actions__c-button--PaddingRight);
+    --pf-c-button--PaddingBottom: var(--pf-c-label-actions__c-button--PaddingBottom);
+    --pf-c-button--PaddingLeft: var(--pf-c-label-actions__c-button--PaddingLeft);
+
+    margin-top: var(--pf-c-label-actions__c-button--MarginTop);
+    margin-right: var(--pf-c-label-actions__c-button--MarginRight);
+    margin-bottom: var(--pf-c-label-actions__c-button--MarginBottom);


Suggested change

--pf-c-button--FontSize: var(--pf-c-label-actions__c-button--FontSize);

--pf-c-button--PaddingTop: var(--pf-c-label-actions__c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-label-actions__c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-label-actions__c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-label-actions__c-button--PaddingLeft);

margin-top: var(--pf-c-label-actions__c-button--MarginTop);

margin-right: var(--pf-c-label-actions__c-button--MarginRight);

margin-bottom: var(--pf-c-label-actions__c-button--MarginBottom);

--pf-c-button--FontSize: var(--pf-c-label-actions--c-button--FontSize);

--pf-c-button--PaddingTop: var(--pf-c-label-actions--c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-label-actions--c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-label-actions--c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-label-actions--c-button--PaddingLeft);

margin-top: var(--pf-c-label-actions--c-button--MarginTop);

margin-right: var(--pf-c-label-actions--c-button--MarginRight);

margin-bottom: var(--pf-c-label-actions--c-button--MarginBottom);

mcoker · 2023-02-07T17:18:16Z

src/patternfly/components/Label/label.scss

-    margin-right: var(--pf-c-label__c-button--MarginRight);
-    margin-bottom: var(--pf-c-label__c-button--MarginBottom);
-    margin-left: var(--pf-c-label__c-button--MarginLeft);
+  .pf-c-label__actions {


can this be moved out of the parent .pf-c-label selector?

mcoker · 2023-02-07T17:18:31Z

src/patternfly/components/Label/label.scss

+    font-size: var(--pf-c-label__actions--FontSize);
+  }
+
+  .pf-c-label__actions .pf-c-button {


can you nest this as .pf-c-button { ... } under .pf-c-label__actions?

mcoker · 2023-02-07T19:38:26Z

src/patternfly/components/Chip/chip-content.hbs

+    {{{chip-content--attribute}}}
+  {{/if}}>
+  {{> @partial-block}}
+</span>


Can you add a newline at the end of the file?

mcoker · 2023-02-07T19:39:14Z

src/patternfly/components/Label/examples/Label.md

 | `.pf-c-label__icon` | `<span>` | Initiates a label icon. |
 | `.pf-c-label__text` | `<span>` | Initiates label text. |
 | `.pf-c-label__editable-text` | `<button>`, `<input>` | Initiates editable label text. See the [editable](#editable) example for details about handling behavior in Javascript.|
+| `.pf-c-label__actions` | `<div>` | Creates a wrapper for label actions. **Required for removable labels** |


mcoker · 2023-02-07T20:05:37Z

src/patternfly/components/Chip/chip.scss

+  --pf-c-chip-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);
+  --pf-c-chip-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);
+  --pf-c-chip-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);
+  --pf-c-chip-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);
+  --pf-c-chip-actions--c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);
+  --pf-c-chip-actions--c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);
+  --pf-c-chip-actions--c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);
+  --pf-c-chip-actions--c-button--FontSize: var(--pf-global--FontSize--xs);


Suggested change

--pf-c-chip-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-chip-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-chip-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-chip-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-chip-actions--c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);

--pf-c-chip-actions--c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);

--pf-c-chip-actions--c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);

--pf-c-chip-actions--c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-chip__actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-chip__actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-chip__actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-chip__actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-chip__actions--c-button--MarginTop: calc(var(--pf-c-chip--PaddingTop) * -1);

--pf-c-chip__actions--c-button--MarginRight: calc(var(--pf-c-chip--PaddingRight) / 2 * -1);

--pf-c-chip__actions--c-button--MarginBottom: calc(var(--pf-c-chip--PaddingBottom) * -1);

--pf-c-chip__actions--c-button--FontSize: var(--pf-global--FontSize--xs);

mcoker · 2023-02-07T20:06:32Z

src/patternfly/components/Chip/chip.scss

+    --pf-c-button--PaddingTop: var(--pf-c-chip-actions--c-button--PaddingTop);
+    --pf-c-button--PaddingRight: var(--pf-c-chip-actions--c-button--PaddingRight);
+    --pf-c-button--PaddingBottom: var(--pf-c-chip-actions--c-button--PaddingBottom);
+    --pf-c-button--PaddingLeft: var(--pf-c-chip-actions--c-button--PaddingLeft);
+    --pf-c-button--FontSize: var(--pf-c-chip-actions--c-button--FontSize);
+
+    margin-top: var(--pf-c-chip-actions--c-button--MarginTop);
+    margin-right: var(--pf-c-chip-actions--c-button--MarginRight);
+    margin-bottom: var(--pf-c-chip-actions--c-button--MarginBottom);


Suggested change

--pf-c-button--PaddingTop: var(--pf-c-chip-actions--c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-chip-actions--c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-chip-actions--c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-chip-actions--c-button--PaddingLeft);

--pf-c-button--FontSize: var(--pf-c-chip-actions--c-button--FontSize);

margin-top: var(--pf-c-chip-actions--c-button--MarginTop);

margin-right: var(--pf-c-chip-actions--c-button--MarginRight);

margin-bottom: var(--pf-c-chip-actions--c-button--MarginBottom);

--pf-c-button--PaddingTop: var(--pf-c-chip__actions--c-button--PaddingTop);

--pf-c-button--PaddingRight: var(--pf-c-chip__actions--c-button--PaddingRight);

--pf-c-button--PaddingBottom: var(--pf-c-chip__actions--c-button--PaddingBottom);

--pf-c-button--PaddingLeft: var(--pf-c-chip__actions--c-button--PaddingLeft);

--pf-c-button--FontSize: var(--pf-c-chip__actions--c-button--FontSize);

margin-top: var(--pf-c-chip__actions--c-button--MarginTop);

margin-right: var(--pf-c-chip__actions--c-button--MarginRight);

margin-bottom: var(--pf-c-chip__actions--c-button--MarginBottom);

mcoker · 2023-02-07T20:07:31Z

src/patternfly/components/Label/label.scss

+  --pf-c-label-actions--MarginRight: calc(var(--pf-global--spacer--form-element) * -1);
+
+  // Actions button (close)
+  --pf-c-label-actions--c-button--FontSize: var(--pf-global--FontSize--xs);
+  --pf-c-label-actions--c-button--MarginTop: calc(var(--pf-global--spacer--form-element) * -1);
+  --pf-c-label-actions--c-button--MarginBottom: calc(var(--pf-global--spacer--form-element) * -1);
+  --pf-c-label-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);
+  --pf-c-label-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);
+  --pf-c-label-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);
+  --pf-c-label-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);


While the margin-right looks ok visually, it's kind of an arbitrary choice in spacer as far as I can tell. It looks like the margins that reference the form-element spacer may just be out of date?

If we want to keep that same visual spacing, I think this formula for that negative margin works better and would scale well? Seems to scale as long as the action's right padding is greater than the right padding of the label.

Suggested change

--pf-c-label-actions--MarginRight: calc(var(--pf-global--spacer--form-element) * -1);

// Actions button (close)

--pf-c-label-actions--c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-label-actions--c-button--MarginTop: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions--c-button--MarginBottom: calc(var(--pf-global--spacer--form-element) * -1);

--pf-c-label-actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-label-actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-label-actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-label-actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

--pf-c-label__actions--MarginRight: calc(var(--pf-c-label-actions--c-button--PaddingRight) * -1 + var(--pf-c-label--PaddingRight) / 2);

// Actions button (close)

--pf-c-label__actions--c-button--FontSize: var(--pf-global--FontSize--xs);

--pf-c-label__actions--c-button--MarginTop: calc(var(--pf-c-label__actions--c-button--PaddingTop) * -1);

--pf-c-label__actions--c-button--MarginBottom: calc(var(--pf-c-label__actions--c-button--PaddingBottom) * -1);

--pf-c-label__actions--c-button--PaddingTop: var(--pf-global--spacer--xs);

--pf-c-label__actions--c-button--PaddingRight: var(--pf-global--spacer--sm);

--pf-c-label__actions--c-button--PaddingBottom: var(--pf-global--spacer--xs);

--pf-c-label__actions--c-button--PaddingLeft: var(--pf-global--spacer--sm);

Just a side note on that margin, ideally I think it could be setup like the label icon instead where the icon touches the start of the label's padding, regardless the size or padding or whatever. I think to do that, the actions margin-right would just be calc(var(--pf-c-label-actions--c-button--PaddingRight) * -1), but that ends up looking a little tight on the spacing, so we could add .fa-fw to the close icons - that class also exists on the label icon. But that would need to be tested in react, too, to make sure it displays correctly.

mcoker · 2023-02-07T20:22:50Z

src/patternfly/components/Label/label.scss

+  margin-right: var(--pf-c-label-actions--MarginRight);
+  font-size: var(--pf-c-label__actions--FontSize);
+
+  & .pf-c-button {


Suggested change

& .pf-c-button {

.pf-c-button {

mcoker · 2023-02-07T20:23:12Z

src/patternfly/components/Chip/chip.scss

+  font-size: var(--pf-c-chip__actions--FontSize);
+
+  // Actions Button
+  & .pf-c-button {


Suggested change

& .pf-c-button {

.pf-c-button {

mcoker · 2023-02-07T20:55:36Z

src/patternfly/components/Label/label.scss

+    margin-top: var(--pf-c-label-actions--c-button--MarginTop);
+    margin-bottom: var(--pf-c-label-actions--c-button--MarginBottom);


do you think it's beneficial to keep these on the button? I'm thinking whatever actions we have in here will all have the same padding, so it might be easier just to put it on the actions container?

Since the padding is on the button (and it needs to be for the clickable area) I think it's best to keep the negative margin on the same element.

mcoker

A couple of comments, but looks like there are a lot of other places chips are used that need to be updated. An alternative could be to make chip.hbs more intelligent like label.hbs so it includes everything needed and can take a chip--id that adds the id and aria-label stuff automatically.

cmichael-mac:patternfly cmichael$ grep -ri '{{#> chip \|{{#> chip}}' src/patternfly/
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:        {{#> chip}}
src/patternfly//components/TextInputGroup/text-input-group--chip-group.hbs:          {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="div"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="div"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="div"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="div"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
# src/patternfly//components/Chip/examples/Chip.md:{{#> chip chip--type="div" chip--modifier="pf-m-draggable"}}
src/patternfly//components/Toolbar/toolbar-item-chip-group.hbs:          {{#> chip}}
src/patternfly//components/Toolbar/toolbar-item-chip-group.hbs:          {{#> chip}}
src/patternfly//components/Toolbar/toolbar-item-chip-group.hbs:          {{#> chip}}
src/patternfly//components/InlineEdit/examples/InlineEdit.md:      {{#> chip chip--type="div" chip--modifier="pf-m-read-only"}}
src/patternfly//components/Select/select-chip-group-collapsed.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-collapsed.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-collapsed.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-collapsed.hbs:        {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
src/patternfly//components/Select/select-chip-group-expanded.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-expanded.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-expanded.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-expanded.hbs:        {{#> chip}}
src/patternfly//components/Select/select-chip-group-expanded.hbs:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip chip--type="button" chip--modifier="pf-m-overflow"}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:        {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//components/ChipGroup/examples/ChipGroup.md:      {{#> chip}}
src/patternfly//demos/DescriptionList/examples/DescriptionList.md:        {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:              {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:              {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:              {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:                  {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:                  {{#> chip}}
src/patternfly//demos/Toolbar/examples/Toolbar.md:                  {{#> chip}}

mcoker · 2023-02-28T00:30:18Z

src/patternfly/components/Chip/examples/Chip.md

+| `.pf-c-button` | `.pf-c-chip__actions <button>` | Initiates the button used to remove the chip. |
+| `.pf-c-badge` | `.pf-c-chip__content <span>` | Initiates the badge inside the chip. |


Just a nit but AFAIK we've moved away from calling other components out in the docs like this, I don't think we need to include them.

mcoker · 2023-02-28T00:30:40Z

src/patternfly/components/Label/examples/Label.md

 | `.pf-c-label__icon` | `<span>` | Initiates a label icon. |
 | `.pf-c-label__text` | `<span>` | Initiates label text. |
 | `.pf-c-label__editable-text` | `<button>`, `<input>` | Initiates editable label text. See the [editable](#editable) example for details about handling behavior in Javascript.|
+| `.pf-c-label__actions` | `<span>` | Creates a wrapper for label actions. **Required for removable labels** |
+| `.pf-c-button` | `.pf-c-label__actions <button>` | Initiates the button used to remove the label. |


same here, I don't think we need this.

mcoker · 2023-02-28T00:48:23Z

src/patternfly/components/Chip/examples/Chip.md

+    {{/chip-text}}
+  {{/chip-content}}
+  {{#> chip-actions}}
+    {{#> button button--modifier="pf-m-plain pf-m-small" button--attribute='aria-labelledby="remove_chip_one chip_one" aria-label="Remove" id="remove_chip_one"'}}


do you need .pf-m-small on this one? I don't see it on any others.

Suggested change

{{#> button button--modifier="pf-m-plain pf-m-small" button--attribute='aria-labelledby="remove_chip_one chip_one" aria-label="Remove" id="remove_chip_one"'}}

{{#> button button--modifier="pf-m-plain" button--attribute='aria-labelledby="remove_chip_one chip_one" aria-label="Remove" id="remove_chip_one"'}}

mcoker

LGTM! Just a few nits.

mcoker · 2023-02-28T21:49:45Z

src/patternfly/components/Chip/examples/Chip.md

+| `.pf-c-chip__content` | `<span>` | Creates a content wrapper for the chip. **Required** |
+| `.pf-c-chip__text` | `<span>` | Initiates the text inside the chip. **Required** |
+| `.pf-c-chip__icon` | `<span>` | Initiates the icon inside the chip. |
+| `.pf-c-chip__actions` | `<span>` | Creates a wrapper for chip actions. **Required for removeable chips** |


mcoker · 2023-02-28T21:50:46Z

src/patternfly/components/ChipGroup/examples/ChipGroup.md

+          {{#> chip-content}}
+						{{#> chip-text chip-text--attribute=(concat 'id="' chip-group--id 'chip_one_select_collapsed"')}}
+              Chip one
+            {{/chip-text}}
+					{{/chip-content}}
+          {{#> chip-actions}}
+						{{#> button button--modifier="pf-m-plain" button--attribute=(concat 'aria-labelledby="' chip-group--id 'remove_chip_one_select_collapsed ' chip-group--id 'chip_one_select_collapsed" aria-label="Remove" id="' chip-group--id 'remove_chip_one_select_collapsed"')}}
+              <i class="fas fa-times" aria-hidden="true"></i>
+          	{{/button}}
+					{{/chip-actions}}


nit - looks like you've got some indentation stuff going on in a lot of blocks in this file.

mcoker · 2023-02-28T21:53:26Z

src/patternfly/components/Toolbar/toolbar-item-chip-group.hbs

+              {{#> button button--modifier="pf-m-plain" button--attribute=(concat 'aria-labelledby="' chip-group--id 'remove-chip-two ' chip-group--id 'chip-two" aria-label="Remove" id="' chip-group--id 'remove-chip-two"')}}
+                <i class="fas fa-times" aria-hidden="true"></i>
+              {{/button}}
+					{{/chip-actions}}


indentation action in this file, too, for all of the {{/chip-actions}} lines

patternfly-build · 2023-03-01T23:23:09Z

🎉 This PR is included in version 5.0.0-alpha.22 🎉

The release is available on:

Your semantic-release bot 📦🚀

…ling embedded components (patternfly#5360)

srambach added 2 commits January 30, 2023 14:19

feat(chip, label): add wrappers to isolate component styles

fa98bc4

feat(chip, label): update documentation

6337b01

srambach requested review from mcoker and mattnolting January 30, 2023 19:49

srambach linked an issue Jan 30, 2023 that may be closed by this pull request

Chip/label updates - scope nested component styles #5199

Closed

mcoker requested changes Feb 7, 2023

View reviewed changes

srambach added 2 commits February 7, 2023 13:42

feat(chip, label): address comments

69712ef

feat(chip, label): move margin onto label actions

47d3e74

srambach requested a review from mcoker February 7, 2023 19:00

mcoker requested changes Feb 7, 2023

View reviewed changes

feat(chip, label): move margin, rename again

55c93c6

srambach requested a review from mcoker February 22, 2023 14:39

mcoker requested changes Feb 28, 2023

View reviewed changes

srambach added 4 commits February 28, 2023 15:47

feat(chip, label): update additional examples

026aaa7

feat(chip, label): update uses in hbs

37f5e67

feat(chip, label): remove legacy example

4932e12

Merge branch 'v5' into 5199-chip-label-wrappers

d1a5cd1

srambach requested a review from mcoker February 28, 2023 21:45

mcoker approved these changes Feb 28, 2023

View reviewed changes

srambach added 2 commits February 28, 2023 17:22

feat(chip, label): fix tabs vs spaces

427c959

feat(chip, label): fix tabs vs spaces take 2

e52cf68

srambach requested a review from mcoker March 1, 2023 21:20

mcoker approved these changes Mar 1, 2023

View reviewed changes

mcoker merged commit 97858c3 into patternfly:v5 Mar 1, 2023

patternfly-build added the released on @alpha label Mar 1, 2023

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

Feat(chip, label): Add wrappers for content and actions to manage sty…

f92e302

…ling embedded components (patternfly#5360)

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request May 18, 2023

Feat(chip, label): Add wrappers for content and actions to manage sty…

54fd3bb

…ling embedded components (patternfly#5360)

MariaAga mentioned this pull request Aug 11, 2023

Bug - Badge no longer has spacing between chip text in chip patternfly/patternfly-react#9483

Closed

mattnolting pushed a commit to mattnolting/patternfly that referenced this pull request Dec 12, 2023

Feat(chip, label): Add wrappers for content and actions to manage sty…

df55d9d

…ling embedded components (patternfly#5360)

srambach deleted the 5199-chip-label-wrappers branch April 6, 2024 02:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(chip, label): Add wrappers for content and actions to manage styling embedded components #5360

Feat(chip, label): Add wrappers for content and actions to manage styling embedded components #5360

srambach commented Jan 30, 2023

patternfly-build commented Jan 30, 2023 •

edited

mcoker left a comment

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

mcoker Feb 7, 2023

srambach Feb 8, 2023

mcoker left a comment

mcoker Feb 28, 2023

mcoker Feb 28, 2023

mcoker Feb 28, 2023

mcoker left a comment

mcoker Feb 28, 2023

mcoker Feb 28, 2023

mcoker Feb 28, 2023

patternfly-build commented Mar 1, 2023

	\| `.pf-c-label__actions` \| `<div>` \| Creates a wrapper for label actions. Required for removeable labels \|
	\| `.pf-c-label__actions` \| `<span>` \| Creates a wrapper for label actions. Required for removable labels \|

		margin-top: var(--pf-c-label-actions--c-button--MarginTop);
		margin-bottom: var(--pf-c-label-actions--c-button--MarginBottom);

		\| `.pf-c-button` \| `.pf-c-chip__actions <button>` \| Initiates the button used to remove the chip. \|
		\| `.pf-c-badge` \| `.pf-c-chip__content <span>` \| Initiates the badge inside the chip. \|

	{{#> button button--modifier="pf-m-plain pf-m-small" button--attribute='aria-labelledby="remove_chip_one chip_one" aria-label="Remove" id="remove_chip_one"'}}
	{{#> button button--modifier="pf-m-plain" button--attribute='aria-labelledby="remove_chip_one chip_one" aria-label="Remove" id="remove_chip_one"'}}

	\| `.pf-c-chip__actions` \| `<span>` \| Creates a wrapper for chip actions. Required for removeable chips \|
	\| `.pf-c-chip__actions` \| `<span>` \| Creates a wrapper for chip actions. Required for removable chips \|

Feat(chip, label): Add wrappers for content and actions to manage styling embedded components #5360

Feat(chip, label): Add wrappers for content and actions to manage styling embedded components #5360

Conversation

srambach commented Jan 30, 2023

patternfly-build commented Jan 30, 2023 • edited

mcoker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcoker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcoker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patternfly-build commented Mar 1, 2023

patternfly-build commented Jan 30, 2023 •

edited