Skip to content

feat(Autosuggest, Combobox): add prop to set HTML attributes on input element #495

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

Merged
merged 4 commits into from
May 4, 2021

Conversation

eszthoff
Copy link
Contributor

@eszthoff eszthoff commented Apr 30, 2021

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. Use imperative, present tense in your commit description ("change", not "changed" or "changes") without uppercases or period (.) at the end.

Checklist

  • The implementation has been manually tested and complies with Textkernel browser support guidelines
  • The implementation complies with accessibility standards.
  • The component has a displayName defined.
  • The component comes with a detailed PropTypes (and defaultProps) definition.
  • Component PropTypes are sufficiently described / documented.
  • There is a story in Storybook.

@@ -151,6 +154,7 @@ export function Autosuggest<S>(props: Props<S>) {
onFocus: onFocusInput,
onKeyDown: (e) => handleInputKeyDown(e, highlightedIndex),
'data-lpignore': true,
...inputAttr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, does this sequence of attributes make sense? You could now theoretically overwrite previous attributes like id etc. Could that break the component?

@eszthoff eszthoff requested a review from mukiienko May 3, 2021 12:07
@eszthoff eszthoff merged commit 87e3bad into master May 4, 2021
@eszthoff eszthoff deleted the 266-add-input-attribute-prop branch May 4, 2021 11:14
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.

3 participants