fix(textfield): icon positioning with field label #2380

jenndiaz · 2023-12-19T01:08:36Z

Description

The position of the textfield validation icons currently does not match design spec when the associated feild label wraps or when a side label is applied.

Changes:

updated stories for Text Field and Text Area to include controls for including a field label changing the field lable text and toggling between top and side label
Add Chromatic coverage for all label options
overflow: hidden removes scroll bar from text-area
adjust css to position icon correctly

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

In storybook for the text area

displayLabel: true, sideLabel: true, and valid: true: the check mark icon is properly positioned
displayLabel: true, sideLabel: true, and invalid: true: the alert icon is properly positioned
displayLabel: true, sideLabel: false and valid: true: the check mark is properly positioned
displayLabel: true, sideLabel: false and invalid: true: the alert icon is properly positioned
in both cases, increasing the label text to wrap does not alter icon position
increasing text in text area, does not cause a scroll bar to appear

In storybook for the text field

displayLabel: true, sideLabel: true, and valid: true: the check mark icon is properly positioned
displayLabel: true, sideLabel: true, and invalid: true: the alert icon is properly positioned
displayLabel: true, sideLabel: false and valid: true: the check mark is properly positioned
displayLabel: true, sideLabel: false and invalid: true: the alert icon is properly positioned
in both cases, increasing the label text to wrap does not alter icon position
The docs site should not have any changes
Chromatic should include kitchen sink approach displaying various label options for text field and text area

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

If components have been modified, VRTs have been run on this branch:

VRTs have been run and looked at.
Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

Before

After

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨

github-actions · 2023-12-19T01:16:09Z

🚀 Deployed on https://pr-2380--spectrum-css.netlify.app

github-actions · 2023-12-19T01:17:07Z

File metrics

Summary

Total size: 3.93 MB^*
Total change (Δ): ⬇ 0.07 KB (-0.00%)
Table reports on changes to a package's main file. Other changes can be found in the collapsed "Details" below.

Package	Size	Δ
textfield	38.60 KB	⬇ 0.02 KB

Details

textfield

File	Head	Base	Δ
index-base.css	37.66 KB	37.68 KB	⬇ 0.02 KB (-0.06%)
index-theme.css	1.52 KB	1.52 KB	-
index-vars.css	38.60 KB	38.62 KB	⬇ 0.02 KB (-0.06%)
index.css	38.60 KB	38.62 KB	⬇ 0.02 KB (-0.06%)
mods.json	2.96 KB	2.96 KB	-
themes/express.css	1.05 KB	1.05 KB	-
themes/spectrum.css	1.05 KB	1.05 KB	-

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

mdt2

Looking great! Just one question in the code.

mdt2 · 2024-01-02T18:57:43Z

components/textfield/index.css

@@ -443,7 +443,7 @@ governing permissions and limitations under the License.
    Sets the opacity to 1 as normalize.css sets an opacity to placeholders
    Details: https://github.com/csstools/normalize.css/blob/main/normalize.css#L297
  */
-	-moz-appearance: textfield;
+	appearance: textfield;


Curious about the changes to appearance throughout this file, and wondering if we should remove the comments where we've removed prefixes?

this is the stylelint rule that removed these: https://stylelint.io/user-guide/rules/property-no-vendor-prefix/

I wonder if that's what we want 🤔 It looks like the vendor prefixes were added 5+ years ago when the project moved to individual package versions...

It seems like we should either:

Thoroughly browser test this PR with the prefixes removed to see if behavior matches prod

Or revert the automatic removal of these and create a card in the backlog to explore further

What do you think?

I agree that it needs closer looking at before they are just removed. I was able to get them back in and added some TODO comments. Writing up a backlog ticket to investigate it further as well.

It looks like the prefixes were reverted again. Maybe try adding --no-verify to your commit?

oop, they are back now

components/textfield/stories/textfield.stories.js

mdt2

Great work, thanks for fixing this!

components/textfield/index.css

castastrophe

Looks great - I noticed the prefixes were stripped again and had a few minor notes on the story format. Thanks!

components/textfield/index.css

components/textfield/stories/template.js

components/textfield/stories/textfield.stories.js

jenndiaz added run_vrt For use on PRs looking to kick off VRT and removed run_vrt For use on PRs looking to kick off VRT labels Dec 20, 2023

jenndiaz marked this pull request as ready for review December 20, 2023 20:49

jenndiaz changed the title ~~fix(textfield): icon positioning with wrapped fieldlabel~~ fix(textfield): icon positioning with field label Dec 21, 2023

jenndiaz requested review from jawinn, pfulton, mdt2 and castastrophe December 21, 2023 20:49

jenndiaz force-pushed the jenndiaz/css-652-text-field-icon-bug branch from 76f75a7 to c81095c Compare December 22, 2023 22:53

jenndiaz marked this pull request as draft January 2, 2024 17:12

mdt2 reviewed Jan 2, 2024

View reviewed changes

jenndiaz marked this pull request as ready for review January 2, 2024 21:56

mdt2 approved these changes Jan 3, 2024

View reviewed changes

jawinn suggested changes Jan 5, 2024

View reviewed changes

components/textfield/index.css Outdated Show resolved Hide resolved

components/textfield/index.css Outdated Show resolved Hide resolved

jawinn approved these changes Jan 9, 2024

View reviewed changes

castastrophe requested changes Jan 17, 2024

View reviewed changes

components/textfield/index.css Outdated Show resolved Hide resolved

components/textfield/stories/template.js Outdated Show resolved Hide resolved

components/textfield/stories/textfield.stories.js Outdated Show resolved Hide resolved

jenndiaz requested a review from castastrophe January 18, 2024 19:48

jenndiaz added the ready-for-review label Jan 29, 2024

jenndiaz force-pushed the jenndiaz/css-652-text-field-icon-bug branch from 551c105 to ae59ace Compare February 13, 2024 16:30

jawinn mentioned this pull request Feb 13, 2024

fix(textfield): remove extra padding on nested label and help #2519

Merged

9 tasks

castastrophe reviewed Feb 14, 2024

View reviewed changes

components/textfield/stories/textfield.stories.js Outdated Show resolved Hide resolved

jenndiaz requested a review from castastrophe February 14, 2024 21:36

mdt2 mentioned this pull request Feb 15, 2024

fix(textarea): drag and grow behaviors #2506

Closed

14 tasks

castastrophe approved these changes Feb 16, 2024

View reviewed changes

castastrophe added the run_vrt For use on PRs looking to kick off VRT label Feb 16, 2024

jenndiaz added 3 commits February 16, 2024 12:58

feat(textfield): add label to story

87b71ce

fix(textfield): removes broken inset

d3b4757

fix(textarea): adjust grid for grows

a5bfbaa

jenndiaz added 12 commits February 16, 2024 12:58

fix(textarea): remove scroll bar

815a767

fix(textarea): side label icon position

8e2e521

fix(textarea): sidelabel icon position

84ee6a1

docs(textfield): add label variants to chromatic

961c315

fix(textfield): adds back args

42b9696

fix(textfield): adds back vendor prefixes

9bf6396

fix(textfield): disable stylelint to add back vendor prefixes

86561c6

fix(textarea): remove scroll behavior changes

ddcf480

refactor(textfield): improves readability of sb controls

a08cab2

fix(fieldlable): adds back vender prefixes

d9671e0

docs(textfield): adjust label size with input size in sb

235aef2

docs(textfield): update isChromatic syntax

77e60e7

castastrophe force-pushed the jenndiaz/css-652-text-field-icon-bug branch from b3fef3e to 77e60e7 Compare February 16, 2024 17:58

castastrophe merged commit 468b426 into main Feb 16, 2024

castastrophe deleted the jenndiaz/css-652-text-field-icon-bug branch February 16, 2024 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(textfield): icon positioning with field label #2380

fix(textfield): icon positioning with field label #2380

Uh oh!

jenndiaz commented Dec 19, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Dec 19, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Dec 19, 2023 •

edited

Loading

textfield

Uh oh!

mdt2 left a comment

Uh oh!

mdt2 Jan 2, 2024

Uh oh!

jenndiaz Jan 2, 2024 •

edited

Loading

Uh oh!

mdt2 Jan 3, 2024

Uh oh!

jenndiaz Jan 3, 2024

Uh oh!

castastrophe Jan 17, 2024

Uh oh!

jenndiaz Jan 18, 2024

Uh oh!

Uh oh!

mdt2 left a comment

Uh oh!

Uh oh!

Uh oh!

castastrophe left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix(textfield): icon positioning with field label #2380

fix(textfield): icon positioning with field label #2380

Uh oh!

Conversation

jenndiaz commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How and where has this been tested?

Validation steps

Regression testing

Screenshots

Before

After

To-do list

Uh oh!

github-actions bot commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

textfield

Uh oh!

mdt2 left a comment

Choose a reason for hiding this comment

Uh oh!

mdt2 Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

jenndiaz Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdt2 Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

jenndiaz Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

castastrophe Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

jenndiaz Jan 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdt2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

castastrophe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jenndiaz commented Dec 19, 2023 •

edited

Loading

github-actions bot commented Dec 19, 2023 •

edited

Loading

github-actions bot commented Dec 19, 2023 •

edited

Loading

jenndiaz Jan 2, 2024 •

edited

Loading