fix: don't render attribute aria-invalid="false"

[HowardBraham]

Explanation

The Input and TextField components both include an aria-invalid="false" attribute.

From the aria-invalid documentation:

Note that since the default value for aria-invalid is false, it is not strictly necessary to add the attribute to input.

Before

        <input
          aria-invalid="false"
          autocomplete="off"
          ...
        />

After

        <input
          autocomplete="off"
          ...
        />

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[georgewrmarshall]

Great catch! I appreciate the focus on accessibility here. I've left a suggestion that aligns with how conditional props are passed in some of our other components e.g

metamask-extension/ui/components/component-library/button-base/button-base.tsx

Lines 57 to 60 in 0fa02c5

	{...(tag === 'button' ? { disabled } : {})}
	{...(href && externalLink
	? { target: '_blank', rel: 'noopener noreferrer' }
	: {})}

Also thought the updates to the snapshot were interesting should the underline props be passed down here I'm not familar with the syntax is. Are they special react or redux props?

[metamaskbot]

Builds ready [cb667dc]

• builds: chrome, firefox
• builds (beta): chrome
• builds (flask): chrome, firefox
• builds (MMI): chrome, firefox
• build viz: Build System
• mv3: Background Module Init Stats
• mv3: UI Init Stats
• mv3: Module Load Stats
• mv3: Bundle Size Stats
• mv2: E2e Actions Stats
• code coverage: Report
• storybook: Storybook
• typescript migration: Dashboard
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4
  • common: 0, 1, 2, 3, 4, 5, 6
  • content-script: 0
  • contentscript: contentscript
  • disable-console: disable-console
  • inpage: inpage
  • policy-load: policy-load
  • sentry-install: sentry-install
  • ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1501 ± 43 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	106	163	132	16	8
		domContentLoaded	1352	1725	1501	89	43
		load	1352	1725	1501	89	43
		domInteractive	1352	1725	1501	89	43

Bundle size diffs

• background: 0 Bytes (0.00%)
• ui: 15 Bytes (0.00%)
• common: 0 Bytes (0.00%)

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (a3a99aa) 68.25% compared to head (865da64) 68.25%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #20939   +/-   ##
========================================
  Coverage    68.25%   68.25%           
========================================
  Files         1006     1006           
  Lines        40184    40186    +2     
  Branches     10742    10744    +2     
========================================
+ Hits         27424    27426    +2     
  Misses       12760    12760           

Files Changed	Coverage Δ
ui/components/component-library/input/input.tsx	100.00% <100.00%> (ø)
...ponents/component-library/text-field/text-field.js	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[georgewrmarshall]

Is typescript complaining about this? I'm wondering because when I look at the storybook build in develop the aria-invalid attribute isn't rendering and there doesn't seem to be any snapshot updates?

[HowardBraham]

@georgewrmarshall haha that's an extremely interesting question about the Storybook!

But the aria-invalid="false" is indeed there in the rendered HTML on the develop branch after #20920 was merged (see screenshot below). And after I rebased this PR on top of develop after #20920, it undoes the snapshot update. (That #20920 snapshot update is what originally made me start working on this.)

I see some evidence that it only updates snapshots if there's a direct code change, and not a change in a 2nd-degree import. Something beyond our current understanding is going on.

[metamaskbot]

Builds ready [865da64]

• builds: chrome, firefox
• builds (beta): chrome
• builds (flask): chrome, firefox
• builds (MMI): chrome, firefox
• build viz: Build System
• mv3: Background Module Init Stats
• mv3: UI Init Stats
• mv3: Module Load Stats
• mv3: Bundle Size Stats
• mv2: E2e Actions Stats
• code coverage: Report
• storybook: Storybook
• typescript migration: Dashboard
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4
  • common: 0, 1, 2, 3, 4, 5, 6
  • content-script: 0
  • contentscript: contentscript
  • disable-console: disable-console
  • inpage: inpage
  • policy-load: policy-load
  • sentry-install: sentry-install
  • ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1657 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	116	190	146	19	9
		domContentLoaded	1491	1820	1657	86	41
		load	1492	1820	1657	86	41
		domInteractive	1491	1820	1657	86	41

Bundle size diffs

• background: 0 Bytes (0.00%)
• ui: 15 Bytes (0.00%)
• common: 0 Bytes (0.00%)

[HowardBraham]

@georgewrmarshall follow-up discovery

• The Default story is passing error: undefined to the components, which makes it skip the aria-invalid attribute
• The Error story is passing error: true, which turns up as aria-invalid="true"
• If you pass error: false, it turns up as aria-invalid="false"

So I think that solves the Storybook mystery. Why it sometimes does not update snapshots, I still don't know.

[georgewrmarshall]

Right on! Thanks for the thorough investigation and explanation as always 🙏 LGTM!

[HowardBraham]

@NidhiKJha @DDDDDanica could one of you review this please? I'd love to close the loop on this PR.

[NidhiKJha]

LGTM