feat(form-control): move background icons to element #5531

srambach · 2023-05-04T13:38:06Z

Note: examples using form control still need to be updated

This changes from using a background image on a form control to using a wrapper around the form control with associated utility container for icon, status icons, and toggle icon.

note: There is some existing code for placeholder text color that doesn't seem to quite work right and the existing feature is not quite as I'd expect so I'd like to discuss @mcoker

patternfly-build · 2023-05-04T13:44:37Z

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

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

mcoker

Looks pretty good to me!

I see .pf-c-form-control--[element] classes on the form elements - are those needed? Generally speaking, I think it would be better to target the class than the html element across the board when we can. Though I believe if we want to keep the class, it should probably be __element - we would use --element in a var to target a generic element that's a child.
Adding a status to any of the text input examples that already have a non-status icon doesn't work as intended, but that can be a follow up if we want.

Now that we have a div wrapping these elements, I think we can move the borders to pseudo elements and not have to adjust heights and paddings and things according to the border width, which would simplify a lot of the CSS. I think it's worth taking a stab at this, wdyt? I'd be happy to run through that, too.

mcoker · 2023-05-09T18:02:03Z

src/patternfly/components/FormControl/form-control.scss

+  display: grid;
+  grid-template-columns: 1fr auto;
+  column-gap: var(--#{$form-control}--ColumnGap);
+  align-items: flex-start;


this works, but since it's grid, I'd just use start since it's well supported.

Suggested change

align-items: flex-start;

align-items: start;

mcoker · 2023-05-09T18:03:35Z

src/patternfly/components/FormControl/form-control.scss

-    height: var(--#{$form-control}--Height);
-
-    // truncate overflow text
+  & > *::placeholder {


Just a nit, the & and * are redundant?

Suggested change

& > *::placeholder {

> ::placeholder {

mcoker · 2023-05-09T18:07:07Z

src/patternfly/components/FormControl/form-control.scss


    // stylelint-disable selector-not-notation
-    // update to single :not() in breaking change
-    &:not(.pf-m-success):not([aria-invalid="true"]) {
+    &:not(.pf-m-success, .pf-m-warning, .pf-m-error,[aria-invalid="true"]) {


You should be able to remove this? That would go on the form element and the border has been moved to the parent.

Suggested change

&:not(.pf-m-success, .pf-m-warning, .pf-m-error,[aria-invalid="true"]) {

&:not(.pf-m-success, .pf-m-warning, .pf-m-error) {

mcoker · 2023-05-09T18:14:17Z

src/patternfly/components/FormControl/form-control.scss

-    background-image: var(--#{$form-control}__select--BackgroundUrl);
-    background-position: var(--#{$form-control}__select--BackgroundPosition);
-    background-size: var(--#{$form-control}__select--BackgroundSize);
+  & > select {


nit - I think the & is redundant?

Suggested change

& > select {

& > select {

mcoker · 2023-05-09T18:15:35Z

src/patternfly/components/FormControl/form-control.scss

-    --#{$form-control}--m-warning--BackgroundPositionY: var(--#{$form-control}--textarea--m-warning--BackgroundPositionY);
-
+
+  & > textarea {


nit - redundant?

Suggested change

& > textarea {

> textarea {

mcoker · 2023-05-09T18:26:05Z

src/patternfly/components/FormControl/form-control.scss

    resize: vertical;
  }

-  &.pf-m-resize-horizontal {
+  &:has(textarea),


This selector won't work in FF or in pre 15.4 safari. Is it needed?

mcoker · 2023-05-09T18:28:09Z

src/patternfly/components/FormControl/form-control-select.hbs

+  <div class="{{pfv}}form-control__toggle-icon">
+  <i class="fas fa-caret-down" aria-hidden="true"></i>
+  </div>


nit

Suggested change

<div class="{{pfv}}form-control__toggle-icon">

<i class="fas fa-caret-down" aria-hidden="true"></i>

</div>

<div class="{{pfv}}form-control__toggle-icon">

<i class="fas fa-caret-down" aria-hidden="true"></i>

</div>

mcoker · 2023-05-09T18:28:26Z

src/patternfly/components/FormControl/form-control-template-status.hbs

+  {{> form-control-icon form-control--HasIcon="exclamation-triangle"}}
+{{else if form-control--IsError}}
+  {{> form-control-icon form-control--HasIcon="exclamation-circle"}}
+{{/if}}


Looks like this file is missing a new line at EOF

mcoker · 2023-05-09T18:29:21Z

src/patternfly/components/FormControl/form-control.scss

@@ -1,33 +1,6 @@
-// @debug $form-control; // check your variable names located in src/patternfly/sass-utilities/component-namespaces.scss
-// Input - success
-$pf-v5-c-form-control--success--Color: $pf-v5-global--success-color--100 !default;


mcoker · 2023-05-09T18:59:55Z

src/patternfly/components/FormControl/form-control.scss

+  padding-right: var(--#{$form-control}__utilities--PaddingRight);
+  padding-left: var(--#{$form-control}__utilities--PaddingLeft);


Just a nit but since this basically just overlays the element behind it, do we need a left padding?

mattnolting

More to come...

mattnolting · 2023-05-09T14:10:36Z

scripts/helpers.mjs

+// export const ifAnyHELP = function () {
+//   const args = Array.prototype.slice.call(arguments, 0, -1);
+//   const options = arguments[arguments.length - 1];
+//   const anyTruthy = args.some(arg => arg);
+//   };
+
+// return anyTruthy ? options.fn(this) : options.inverse(this);
+// };


Did you intend to leave this in?

mattnolting · 2023-05-09T14:10:44Z

src/patternfly/components/FormControl/form-control-icon.hbs

+    {{{form-control-icon--attribute}}}
+  {{/if}}>
+  <i class="fas fa-{{form-control--HasIcon}}" aria-hidden="true"></i>
+</div>


Suggested change

</div>

</div>

Just want to make sure you saw this, this file is missing a new line at EOF

mattnolting · 2023-05-09T14:11:02Z

src/patternfly/components/FormControl/form-control-template-status.hbs

+  {{> form-control-icon form-control--HasIcon="exclamation-triangle"}}
+{{else if form-control--IsError}}
+  {{> form-control-icon form-control--HasIcon="exclamation-circle"}}
+{{/if}}


Suggested change

{{/if}}

{{/if}}

mattnolting · 2023-05-11T14:29:42Z

src/patternfly/components/FormControl/form-control.hbs

+  {{#if form-control--IsExpanded}} pf-m-expanded{{/if}}
+  {{#if form-control--IsDisabled}} pf-m-disabled{{/if}}
+  {{#if form-control--IsReadonly}} pf-m-readonly{{/if}}
+  {{#if form-control--IsRequired}} pf-m-required{{/if}}
+  {{#if form-control--IsPlaceholder}} pf-m-placeholder{{/if}}
+  {{#if form-control--IsPlain}} pf-m-plain{{/if}}
+  {{#if form-control--IsSuccess}} pf-m-success{{/if}}
+  {{#if form-control--IsWarning}} pf-m-warning{{/if}}
+  {{#if form-control--IsError}} pf-m-error{{/if}}
+  {{#if form-control--HasIcon}} pf-m-icon{{/if}}
+  {{#if form-control--modifier}} {{form-control--modifier}}{{/if}}


👍 Love this update!

thatblindgeye · 2023-05-11T18:09:43Z

src/patternfly/components/FormControl/examples/FormControl.md


 ### Usage
 | Class | Applied to | Outcome |
 | -- | -- | -- |
-| `.pf-v5-c-form-control` | `<input>`,`<textarea>`, `<select>` |  Initiates an input, textarea or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required**  |
+| `.pf-v5-c-form-control` | `<div>` |  Initiates a container for an input, textarea or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required**  |
+| `.pf-v5-c-form-control` | `<input>`,`<textarea>`, `<select>` |  Initiates an input, textarea or select.  **Required**  |


Is the intent that either a div or input/textarea/select can have this class? Based on the examples it seems like line 172 here should be removed.

thatblindgeye · 2023-05-11T18:12:18Z

src/patternfly/components/FormControl/examples/FormControl.md

-| `.pf-m-placeholder` | `select.pf-v5-c-form-control` | Modifies a form select for placeholder styles. This modifier is set programatically based on the chosen option. |
-| `.pf-m-plain` | `input[readonly].pf-v5-c-form-control`, `textarea[readonly].pf-v5-c-form-control` | Modifies an `<input>` or `<textarea>` with a `readonly` attribute to be presented as normal text. |
+| `.pf-m-error` | `.pf-v5-c-form-control` | Modifies a form control for the error (invalid) state. |
+| `.pf-v5-c-form-control__icon` | `div` | Creates a container for an icon associated with a text input.


Looks like .pf-v5-c-form-control__utilities is missing from the docs, same for pf-m-status and pf-m-icon. One other nit:

Suggested change

| `.pf-v5-c-form-control__icon` | `div` | Creates a container for an icon associated with a text input.

| `.pf-v5-c-form-control__icon` | `div` | Creates a container for an icon associated with a form control.

thatblindgeye · 2023-05-11T18:30:00Z

src/patternfly/components/FormControl/form-control.scss

  --#{$form-control}--invalid--BorderBottomWidth: var(--#{$pf-global}--BorderWidth--md);
  --#{$form-control}--invalid--PaddingBottom: calc(var(--#{$pf-global}--spacer--form-element) - var(--#{$form-control}--invalid--BorderBottomWidth));
  --#{$form-control}--invalid--BorderBottomColor: var(--#{$pf-global}--danger-color--100);
  --#{$form-control}--invalid--PaddingRight: var(--#{$pf-global}--spacer--xl);


Not a blocker for this PR, but @mcoker would it make sense to update these --invalid vars to ...--m-invalid?

@thatblindgeye yep, nice catch. Since it's a modifier now, this would be --m-invalid

I've been debating on this. It could also be --m-error

mcoker

Just some nits, none are blocking. I say we merge and follow up?

Also I think this needs to be &.pf-m-readonly?

patternfly/src/patternfly/components/FormControl/themes/dark/form-control.scss

Lines 21 to 23 in 4455d9b

    
           &[readonly] { 
        
             border-bottom-color: var(--#{$pf-global}--palette--black-700); // should be a var? 
        
           }

mcoker · 2023-05-11T19:43:06Z

src/patternfly/components/FormControl/examples/FormControl.md

-| `.pf-m-resize-horizontal` | `textarea.pf-m-form-control` | Modifies a `textarea.pf-v5-c-form-control` element so it can only be resized horizontally along the x-axis. |
+| `.pf-v5-c-form-control` | `<div>` |  Initiates a container for an input, textarea or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required**  |
+| `.pf-v5-c-form-control__utilities` | `<div>` |  Initiates a container for elements like icons to be associated with the form control.  |
+| `.pf-v5-c-form-control__icon` | `<div>` |  Creates a container for an icon associated with a text input.  |


Since this element is used for status icons, that applies to any control.

Suggested change

| `.pf-v5-c-form-control__icon` | `<div>` | Creates a container for an icon associated with a text input. |

| `.pf-v5-c-form-control__icon` | `<div>` | Creates a container for an icon associated with a form control. |

mcoker · 2023-05-11T19:44:37Z

src/patternfly/components/FormControl/examples/FormControl.md

+| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a `pf-v5-c-form-control` element containing a text area so it can only be resized vertically. |
+| `.pf-m-resize-horizontal` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized horizontally. |
+| `.pf-m-resize-both` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it resizes in both directions. |
+| `.pf-m-icon` | `.pf-v5-c-form-control` | Modifies a form control to allow for an icon. |
+| `.pf-m-readonly` | `.pf-v5-c-form-control` | Modifies a form control for a readonly input, textarea, or select.|
+| `.pf-m-disabled` | `.pf-v5-c-form-control` | Modifies a form control for a disabled input, textarea, or select.|


You could probably update the wording of "Modifies a .pf-v5-c-form-control element" to "Modifies a form control" - that seems to be the wording in the other docs descriptions.

Suggested change

| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a `pf-v5-c-form-control` element containing a text area so it can only be resized vertically. |

| `.pf-m-resize-horizontal` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized horizontally. |

| `.pf-m-resize-both` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it resizes in both directions. |

| `.pf-m-icon` | `.pf-v5-c-form-control` | Modifies a form control to allow for an icon. |

| `.pf-m-readonly` | `.pf-v5-c-form-control` | Modifies a form control for a readonly input, textarea, or select.|

| `.pf-m-disabled` | `.pf-v5-c-form-control` | Modifies a form control for a disabled input, textarea, or select.|

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized vertically. |

| `.pf-m-resize-horizontal` | `.pf-v5-c-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized horizontally. |

| `.pf-m-resize-both` | `.pf-v5-c-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it resizes in both directions. |

| `.pf-m-icon` | `.pf-v5-c-form-control` | Modifies a form control to allow for an icon. |

| `.pf-m-readonly` | `.pf-v5-c-form-control` | Modifies a form control for a readonly input, textarea, or select.|

| `.pf-m-disabled` | `.pf-v5-c-form-control` | Modifies a form control for a disabled input, textarea, or select.|

mcoker · 2023-05-11T19:45:39Z

src/patternfly/components/FormControl/examples/FormControl.md

+| `.pf-m-expanded` | `.pf-v5-c-form-control` | Modifies a form control for the expanded state. This is used when clicking in the text input toggles something open/closed. |
+| `.pf-m-placeholder` | `.pf-v5-c-form-control` | Modifies a form select for placeholder styles. This modifier is set programatically based on the chosen option. |
+| `.pf-m-plain` | `.pf-v5-c-form-control` | Modifies a form control containing an `<input>` or `<textarea>` with a `readonly` attribute to be presented as normal text. |
+| `.pf-m-status`| `.pf-v5-c-form-control__icon` | Modified a form control icon to show status.


mcoker · 2023-05-11T20:07:53Z

src/patternfly/components/FormControl/form-control.scss

+    --#{$form-control}--PaddingRight: var(--#{$form-control}--m-success--PaddingRight);
+    --#{$form-control}--PaddingBottom: var(--#{$form-control}--m-success--PaddingBottom);
+    --#{$form-control}--BorderBottomColor: var(--#{$form-control}--m-success--BorderBottomColor);
+    --#{$form-control}__icon--m-status--Color: var(--#{$form-control}--m-success__icon--Color);


This should be updated for the other states, too

Suggested change

--#{$form-control}__icon--m-status--Color: var(--#{$form-control}--m-success__icon--Color);

--#{$form-control}__icon--m-status--Color: var(--#{$form-control}--m-success__icon--m-status--Color);

mcoker · 2023-05-11T20:11:55Z

src/patternfly/components/FormControl/form-control.scss

+  &.pf-m-disabled {
    --#{$form-control}--BackgroundColor: var(--#{$form-control}--disabled--BackgroundColor);
+    --#{$form-control}--Color: var(--#{$form-control}--disabled--Color);
+    --#{$form-control}__toggle-icon--Color: var(--#{$form-control}--disabled__toggle-icon--Color);


Just tagged a few here, but I think there are others that should be updated to --m-disabled

Suggested change

&.pf-m-disabled {

--#{$form-control}--BackgroundColor: var(--#{$form-control}--disabled--BackgroundColor);

--#{$form-control}--Color: var(--#{$form-control}--disabled--Color);

--#{$form-control}__toggle-icon--Color: var(--#{$form-control}--disabled__toggle-icon--Color);

&.pf-m-disabled {

--#{$form-control}--BackgroundColor: var(--#{$form-control}--m-disabled--BackgroundColor);

--#{$form-control}--Color: var(--#{$form-control}--m-disabled--Color);

--#{$form-control}__toggle-icon--Color: var(--#{$form-control}--m-disabled__toggle-icon--Color);

import

mattnolting

LPTM 👍

mattnolting · 2023-05-11T14:42:23Z

src/patternfly/components/FormControl/form-control-select.hbs

+  {{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning }}
+    {{> form-control-template-status}}
+  {{/ifAny}}


👍👍👍

mattnolting · 2023-05-11T14:42:56Z

src/patternfly/components/FormControl/form-control-select.hbs

+    {{/if}}
+>{{#if @partial-block}}{{> @partial-block}}{{/if}}</select>
+{{#> form-control-utilities}}
+  {{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning }}


Suggested change

{{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning }}

{{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning}}

mattnolting · 2023-05-11T14:50:28Z

src/patternfly/components/FormControl/form-control-template-status.hbs

@@ -0,0 +1,7 @@
+{{#if form-control--IsSuccess}}
+  {{> form-control-icon form-control-icon--IsStatus="true" form-control--HasIcon="check-circle"}}


Nit: any bool value doesn't need quotation marks. I know that's inconsistent with most other components, but I'd like to move toward that. And I think we can start when making updates.

Second Nit: As our .hbs functions library grows and our components become more sophisticated, our string values may need to contain double quotes. Example being that we are starting to allow components and/or partials to pass text parameters. component--text='Hello' Can we use single quotes '' as a standard for string values?
Wdty @mcoker

Suggested change

{{> form-control-icon form-control-icon--IsStatus="true" form-control--HasIcon="check-circle"}}

{{> form-control-icon form-control-icon--IsStatus=true form-control--HasIcon='check-circle'}}

@mattnolting that all sounds good to me, with the work you've done looking at how our templates can improve, maybe we can meet and go over your suggestions and make changes like this and and make sure it's documented?

mcoker

A couple of things that need updated, but it's all CSS so we can merge and follow up later if you'd prefer.

mcoker · 2023-05-11T21:08:57Z

src/patternfly/components/FormControl/examples/FormControl.md

 | `.pf-v5-c-form-control__toggle-icon` | `<div>` |  Initiates a toggle icon for a form select.  |
-| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a `pf-v5-c-form-control` element containing a text area so it can only be resized vertically. |
-| `.pf-m-resize-horizontal` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized horizontally. |
+| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |


mcoker · 2023-05-11T21:09:18Z

src/patternfly/components/FormControl/examples/FormControl.md

 | `.pf-v5-c-form-control__toggle-icon` | `<div>` |  Initiates a toggle icon for a form select.  |
-| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a `pf-v5-c-form-control` element containing a text area so it can only be resized vertically. |
-| `.pf-m-resize-horizontal` | `.pf-m-form-control` | Modifies a `.pf-v5-c-form-control` element containing a textarea so it can only be resized horizontally. |
+| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |


Suggested change

| `.pf-m-resize-vertical` | `.pf-m-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a textarea so it can only be resized vertically. |

mcoker · 2023-05-11T21:11:20Z

src/patternfly/components/FormControl/examples/FormControl.md

+| `.pf-v5-c-form-control__utilities` | `<div>` |  Initiates a container for elements like icons to be associated with the form control.  |
+| `.pf-v5-c-form-control__icon` | `<div>` |  Creates a container for an icon associated with a form control.  |
+| `.pf-v5-c-form-control__toggle-icon` | `<div>` |  Initiates a toggle icon for a form select.  |
+| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |


Suggested change

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a textarea so it can only be resized vertically. |

mcoker · 2023-05-11T21:11:28Z

src/patternfly/components/FormControl/examples/FormControl.md

+| `.pf-v5-c-form-control__utilities` | `<div>` |  Initiates a container for elements like icons to be associated with the form control.  |
+| `.pf-v5-c-form-control__icon` | `<div>` |  Creates a container for an icon associated with a form control.  |
+| `.pf-v5-c-form-control__toggle-icon` | `<div>` |  Initiates a toggle icon for a form select.  |
+| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |


Suggested change

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a text area so it can only be resized vertically. |

| `.pf-m-resize-vertical` | `.pf-v5-c-form-control` | Modifies a form control element containing a textarea so it can only be resized vertically. |

mcoker · 2023-05-11T21:11:54Z

src/patternfly/components/FormControl/examples/FormControl.md

-| `.pf-v5-c-form-control` | `<input>`,`<textarea>`, `<select>` |  Initiates an input, textarea or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required**  |
-| `.pf-m-resize-vertical` | `textarea.pf-m-form-control` | Modifies a `textarea.pf-v5-c-form-control` element so it can only be resized vertically along the y-axis. |
-| `.pf-m-resize-horizontal` | `textarea.pf-m-form-control` | Modifies a `textarea.pf-v5-c-form-control` element so it can only be resized horizontally along the x-axis. |
+| `.pf-v5-c-form-control` | `<div>` |  Initiates a container for an input, text area or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required**  |


Suggested change

| `.pf-v5-c-form-control` | `<div>` | Initiates a container for an input, text area or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required** |

| `.pf-v5-c-form-control` | `<div>` | Initiates a container for an input, textarea, or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). **Required** |

mcoker · 2023-05-11T21:25:22Z

src/patternfly/components/FormControl/form-control.scss

-  --#{$form-control}--disabled--Color: var(--#{$pf-global}--disabled-color--100);
-  --#{$form-control}--disabled--BackgroundColor: var(--#{$pf-global}--disabled-color--300);
-  --#{$form-control}--disabled--BorderColor: transparent;
+  --#{$form-control}--m-disabled--Color: var(--#{$pf-global}--m-disabled-color--100);


The global vars need the m- removed, since that will break the styling.

Suggested change

--#{$form-control}--m-disabled--Color: var(--#{$pf-global}--m-disabled-color--100);

--#{$form-control}--m-disabled--Color: var(--#{$pf-global}--disabled-color--100);

mcoker · 2023-05-11T21:26:46Z

src/patternfly/components/FormControl/form-control.scss

+  position: relative;
+  display: grid;
+  grid-template-columns: 1fr auto;
+  column-gap: var(--#{$form-control}--ColumnGap);


Not sure if you saw this. Is the gap used anywhere?

mcoker · 2023-05-11T21:34:39Z

src/patternfly/components/FormControl/form-control.scss

-      background-image: var(--#{$form-control}__select--BackgroundUrl), var(--#{$form-control}--invalid--BackgroundUrl);
-      background-position: var(--#{$form-control}__select--BackgroundPosition), var(--#{$form-control}--invalid--BackgroundPosition);
-      background-size: var(--#{$form-control}__select--BackgroundSize), var(--#{$form-control}--invalid--BackgroundSize);
+    .pf-m-error {


The select state modifiers need & - the padding is broken

Suggested change

.pf-m-error {

&.pf-m-error {

mcoker · 2023-05-11T21:35:48Z

src/patternfly/components/FormControl/form-control.scss

-      background-position: var(--#{$form-control}__select--BackgroundPosition), var(--#{$form-control}--invalid--BackgroundPosition);
-      background-size: var(--#{$form-control}__select--BackgroundSize), var(--#{$form-control}--invalid--BackgroundSize);
+    .pf-m-error {
+      --#{$form-control}--PaddingRight: var(--#{$form-control}--select--m-error--PaddingRight);


These vars need --m-[status] after the base form control class in the name.

Suggested change

--#{$form-control}--PaddingRight: var(--#{$form-control}--select--m-error--PaddingRight);

--#{$form-control}--PaddingRight: var(--#{$form-control}--m-error--select--PaddingRight);

mcoker · 2023-05-11T21:37:43Z

src/patternfly/components/FormControl/form-control-icon.hbs

+    {{{form-control-icon--attribute}}}
+  {{/if}}>
+  <i class="fas fa-{{form-control--HasIcon}}" aria-hidden="true"></i>
+</div>


Just want to make sure you saw this, this file is missing a new line at EOF

mcoker

A couple of things that need updated, but it's all CSS so we can merge and follow up later if you'd prefer.

mcoker

🚀

patternfly-build · 2023-05-15T20:34:35Z

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

The release is available on:

Your semantic-release bot 📦🚀

This was linked to issues May 4, 2023

Spike: investigate solutions for displaying icons in validated text inputs #2702

Closed

Bug - Form and Form control - Error states + colors need changing #4900

Closed

srambach force-pushed the 5469-move-bg-images-to-element branch from 8f8203a to f081220 Compare May 9, 2023 13:01

srambach changed the title ~~5469 move bg images to element~~ feat(form-control): move background icons to element May 9, 2023

srambach requested review from mcoker, mattnolting and thatblindgeye May 9, 2023 13:20

mcoker requested changes May 9, 2023

View reviewed changes

mattnolting reviewed May 11, 2023

View reviewed changes

srambach requested review from mattnolting and mcoker May 11, 2023 15:32

thatblindgeye reviewed May 11, 2023

View reviewed changes

srambach requested a review from thatblindgeye May 11, 2023 19:45

mcoker approved these changes May 11, 2023

View reviewed changes

thatblindgeye approved these changes May 11, 2023

View reviewed changes

srambach added 9 commits May 11, 2023 16:29

feat(form-control): restructure removing bg icons

45bc807

feat(form-control): remove comment

00d4f8c

feat(form-control): fix merge

42671a9

feat(form-control): address quick comment items

b14a400

feat(form-control): add pf-m-status to icon

8c027da

feat(form-control): remove form control

851aaca

import

feat(form-control): remove old comment

c3a9ee4

feat(form-control): docs and better resize

75353f8

feat(form-control): rename invalid to error

0dca624

srambach force-pushed the 5469-move-bg-images-to-element branch from 4455d9b to 0dca624 Compare May 11, 2023 20:32

mattnolting approved these changes May 11, 2023

View reviewed changes

srambach added 2 commits May 11, 2023 17:03

feat(form-control): fix some nits

708acc0

feat(form-control): fix one more nit

ceecc1f

mcoker approved these changes May 11, 2023

View reviewed changes

feat(form-control): placeholder, disabled, select right padding

2f05d20

mcoker approved these changes May 15, 2023

View reviewed changes

mcoker merged commit bcd493a into patternfly:v5 May 15, 2023
1 check passed

patternfly-build added the released on @alpha label May 15, 2023

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

feat(form-control): move background icons to element (patternfly#5531)

2ea2ff1

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

feat(form-control): move background icons to element (patternfly#5531)

132ed29

mcoker mentioned this pull request Mar 5, 2024

fix(FormControl): updated select styling in light and dark themes #6397

Merged

srambach deleted the 5469-move-bg-images-to-element branch April 6, 2024 02:21

	&:not(.pf-m-success, .pf-m-warning, .pf-m-error,[aria-invalid="true"]) {
	&:not(.pf-m-success, .pf-m-warning, .pf-m-error) {

		--#{$form-control}--m-warning--BackgroundPositionY: var(--#{$form-control}--textarea--m-warning--BackgroundPositionY);


		& > textarea {

		padding-right: var(--#{$form-control}__utilities--PaddingRight);
		padding-left: var(--#{$form-control}__utilities--PaddingLeft);

	\| `.pf-v5-c-form-control__icon` \| `div` \| Creates a container for an icon associated with a text input.
	\| `.pf-v5-c-form-control__icon` \| `div` \| Creates a container for an icon associated with a form control.

	&[readonly] {
	border-bottom-color: var(--#{$pf-global}--palette--black-700); // should be a var?
	}

	\| `.pf-v5-c-form-control__icon` \| `<div>` \| Creates a container for an icon associated with a text input. \|
	\| `.pf-v5-c-form-control__icon` \| `<div>` \| Creates a container for an icon associated with a form control. \|

	\| `.pf-m-status`\| `.pf-v5-c-form-control__icon` \| Modified a form control icon to show status.
	\| `.pf-m-status`\| `.pf-v5-c-form-control__icon` \| Modifies a form control icon to show status. \|

	--#{$form-control}__icon--m-status--Color: var(--#{$form-control}--m-success__icon--Color);
	--#{$form-control}__icon--m-status--Color: var(--#{$form-control}--m-success__icon--m-status--Color);

	{{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning }}
	{{#ifAny form-control--HasIcon form-control--IsError form-control--IsSuccess form-control--IsWarning}}

		@@ -0,0 +1,7 @@
		{{#if form-control--IsSuccess}}
		{{> form-control-icon form-control-icon--IsStatus="true" form-control--HasIcon="check-circle"}}

	\| `.pf-m-resize-vertical` \| `.pf-m-form-control` \| Modifies a form control element containing a text area so it can only be resized vertically. \|
	\| `.pf-m-resize-vertical` \| `.pf-m-form-control` \| Modifies a form control element containing a textarea so it can only be resized vertically. \|

	\| `.pf-m-resize-vertical` \| `.pf-m-form-control` \| Modifies a form control element containing a text area so it can only be resized vertically. \|
	\| `.pf-m-resize-vertical` \| `.pf-v5-c-form-control` \| Modifies a form control element containing a textarea so it can only be resized vertically. \|

	\| `.pf-v5-c-form-control` \| `<div>` \| Initiates a container for an input, text area or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). Required \|
	\| `.pf-v5-c-form-control` \| `<div>` \| Initiates a container for an input, textarea, or select. For styling of checkboxes or radios see the [checkbox component](/components/checkbox) or [radio component](/components/radio). Required \|

	--#{$form-control}--m-disabled--Color: var(--#{$pf-global}--m-disabled-color--100);
	--#{$form-control}--m-disabled--Color: var(--#{$pf-global}--disabled-color--100);

	--#{$form-control}--PaddingRight: var(--#{$form-control}--select--m-error--PaddingRight);
	--#{$form-control}--PaddingRight: var(--#{$form-control}--m-error--select--PaddingRight);

feat(form-control): move background icons to element #5531

feat(form-control): move background icons to element #5531

Conversation

srambach commented May 4, 2023

patternfly-build commented May 4, 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

mattnolting 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

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

mattnolting 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

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

mcoker left a comment

Choose a reason for hiding this comment

mcoker left a comment

Choose a reason for hiding this comment

patternfly-build commented May 15, 2023

patternfly-build commented May 4, 2023 •

edited