fix(form-control): move borders to pseudo element, remove extra css

[mcoker]

Updates the form control to move the borders to pseudo elements, remove all of the custom height and padding calc()s that were needed to offset the borders since they were part of the component layout.

[patternfly-build]

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

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

[thatblindgeye]

🎉

[thatblindgeye]

One small thing, doesn't look like this after--BorderWidth var is being used anywhere

[srambach]

FWIW, the focus ring is narrower than it was previously - but now you can see the status color bottom border. It also removes the little artifact at the corner, which is nice.

[srambach]

I think on line 90, --#{$form-control}--textarea--Height can also be removed?

Similar to the previous comment, the focus ring is also cut off when next to something, like in the date picker.

[srambach]

Generally this looks good but for a few nits and the focus ring question.
Also, just for discussion - using --after is consistent with our model, but when I was refactoring the cards we talked about using more logical names to keep future things non-breaking and perhaps ease token use. E.g., if we kept the border variables as is, we could move them between the wrapper or the pseudo-element or whatever clever thing we came up with, without causing a breaking change. WDYT?

[srambach]

Search and replace oops? (--after--after)

[mcoker]

@srambach thanks for the review! Should have addressed everything but the focus ring. Dark theme vars also needed to be updated, moved the :focus styles to :focus-within, and fixed focus styles appearing incorrectly in the readonly input.

Re: the focus ring... a few thoughts. I'll note that from glancing, the focus ring looks the same in menu toggle and control buttons, which have the same look.

• The lack of focus ring date picker is from the input group. The input group moves any * + * child to the left by 1px, to prevent double-borders when putting form-things side by side. So if you focus on anything that has a sibling after it in the input group layout, the sibling component will be on top of the focused component. We had this style previously to address that, we could add it back. wdyt? cc @mattnolting

  patternfly/src/patternfly/components/InputGroup/input-group.scss

  Lines 43 to 46 in 7ddc2de

  	> :focus,
  	> :focus-within {
  	z-index: var(--pf-c-input-group--child--ZIndex);
  	}

• One solution could be to adjust the outline-offset - that would move the focus ring inside - it would need to be inside now because the textarea controls overflow with overflow: auto; and that will hide the outline if it goes outside of the box.

• An easier solution could be to manage the stacking context:

  • move the border to ::before and add position: relative or isolation: isolate to the input/textarea/select
  • keep the borders on ::after and add our xs z-index on the form element. I'd probably prefer this as a solution - ideally I think the grey borders for the form control should be on the ::before and the status/bottom border should be on ::after so they can be drawn separately and not create an angled corner like they do now. This is how the menu toggle is setup currently.

[mattnolting]

I think moving the borders to a pseudo is a good idea, but typically we add gray, static borders to ::before and accent/hover borders to ::after to avoid bevelled borders

[srambach]

I don't hate moving the outline-offset in about 2px. That gives the full focus ring, the width is consistent with other focus rings other than the ones you mention (which then could be updated as well), and still shows the status bottom border color. Do you know of any problems it creates?

[srambach]

If the text area is resized, the focus ring isn't stretching properly.

[srambach]

👍

[mattnolting]

Thanks for updating, LPTM!!

[patternfly-build]

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

The release is available on:

• GitHub release
• npm package (@alpha dist-tag)

Your semantic-release bot 📦🚀