Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(web-components) Use ElementInternals for TextInput elements #31201

Merged

Conversation

radium-v
Copy link
Contributor

Previous Behavior

Currently, form-associated custom elements use the FormAssociated class mixin. This mixin is heavy and duplicative, being copied wholesale for every component which utilizes it. While FormAssociated includes minimal support for ElementInternals, the implementation isn't fully baked.

New Behavior

Improvements:

  • Switches the <fluent-text-input> custom element to use ElementInternals by default. An optional polyfill may be added in the near future, as more components transition to use ElementInternals.
  • Adds additional tests for the TextInput component.
  • Expands the properties and attributes for the Text Input storybook fixtures.

Copy link

codesandbox-ci bot commented Apr 25, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@chrisdholt
Copy link
Member

Should we pull the label from within the control with this change?

@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch from 64a35b4 to 070313f Compare April 26, 2024 18:12
@radium-v radium-v force-pushed the web-components-v3 branch 2 times, most recently from 707be68 to 3ba33e2 Compare May 8, 2024 00:35
@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch from 070313f to af1d832 Compare May 8, 2024 01:03
@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch from af1d832 to e20f6ef Compare May 13, 2024 21:29
@chrisdholt
Copy link
Member

This will need to be pointed to master as we've merged the v3 branch :)

@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch from e20f6ef to 3af5dd9 Compare May 14, 2024 17:24
@radium-v radium-v changed the base branch from web-components-v3 to master May 14, 2024 17:24
@radium-v radium-v marked this pull request as ready for review May 14, 2024 17:25
@radium-v radium-v requested a review from a team as a code owner May 14, 2024 17:25
@fabricteam
Copy link
Collaborator

fabricteam commented May 14, 2024

📊 Bundle size report

✅ No changes found

@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch 2 times, most recently from 3f8fe39 to ccc025b Compare May 17, 2024 22:10
@radium-v
Copy link
Contributor Author

Should we pull the label from within the control with this change?

@chrisdholt due to cross-root ARIA limitations, the label has to be part of the component.

I gave a custom contenteditable approach a shot (https://github.com/radium-v/fluentui/tree/users/radium-v/web-components-v3-text-input-contenteditable), but it would limit our ability to use different types like email and url without specific implementations. I don't trust that any custom alternative to <input type=password> would be as secure as the native control.

@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch 2 times, most recently from 5b79d8d to ce3b7af Compare May 23, 2024 19:37
@radium-v radium-v force-pushed the users/radium-v/web-components-v3-text-input branch from ce3b7af to d5d078b Compare May 23, 2024 22:07
@radium-v radium-v enabled auto-merge (squash) May 23, 2024 22:12
@radium-v radium-v merged commit db8e9c5 into microsoft:master May 23, 2024
16 of 19 checks passed
@radium-v radium-v deleted the users/radium-v/web-components-v3-text-input branch May 23, 2024 22:16
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request May 28, 2024
* master: (49 commits)
  Update focus order in sankey chart for vertical navigation (microsoft#31469)
  chore: use new performant 'type-check' for v9 libaries (microsoft#31454)
  applying package updates
  fix(Timepicker-compat): clearIcon not working in freeform (microsoft#31324)
  chore: re-enable lint rule (microsoft#31459)
  feat(react-tag-picker): adds text property to TagPickerOption (microsoft#31474)
  feat(recipes): create package with initial implementation (moved from /apps) (microsoft#31386)
  applying package updates
  applying package updates
  chore: revert globals changes (microsoft#31470)
  (web-components) Use `ElementInternals` for TextInput elements (microsoft#31201)
  chore:(docs) Update and migrate component implementation guide (microsoft#31398)
  disallow all globals in Fluent v9 (microsoft#30967)
  chore:(react-nav-preview) Recomposing more components and some pixel pushing (microsoft#31387)
  fix(pr-deploy-site): explicitly set types to not include whole @types/* globals which are causing issues with addition of @types/web (microsoft#31465)
  fix(recipes-react-components): explicitly set types to not include whole @types/* globals which are cauising issues with addition of @types/web (microsoft#31463)
  applying package updates
  applying package updates
  applying package updates
  Update IconDirectionContextProvider import to import from specific path (microsoft#31006)
  ...
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants