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

feat(input, input-text, input-number): add attributes autocomplete, accept, multiple, pattern #5807

Merged
merged 4 commits into from
Nov 28, 2022

Conversation

benelan
Copy link
Member

@benelan benelan commented Nov 23, 2022

Related Issue: #4079

Summary

Adds native input attributes:

  • multiple to calcite-input for the types specified on MDN
  • accept to calcite-input for the types specified on MDN
  • pattern to calcite-input-text and calcite-input for the types specified on MDN
  • autocomplete to calcite-input-number, calcite-input-text, and calcite-input for the types specified on MDN

Note

just saw on MDN after committing that Safari may not support autocomplete:
https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete#browser_compatibility

Should I remove it from the PR?

@benelan benelan requested a review from a team as a code owner November 23, 2022 23:03
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Nov 23, 2022
*
* @mdn [step](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete)
*/
@Prop({ reflect: true }) autocomplete?: "string";
Copy link
Member

Choose a reason for hiding this comment

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

@benelan shouldn't it be autocomplete: string;?

We don't need the optional question mark and the type should be string not "string" for these right?

Copy link
Member

Choose a reason for hiding this comment

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

Do these need to be reflected?

Copy link
Member Author

@benelan benelan Nov 23, 2022

Choose a reason for hiding this comment

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

Woops my bad, fixing. I copy and pasted from the step property which has "any" in quotes for some reason. I'll fix that in a separate PR
Nevermind I remembered step is supposed to be like that

Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

Looks good!

Do you think it would beneficial to make sure these are passed to their internal inputs via tests? if not i'm ok with that.

@benelan
Copy link
Member Author

benelan commented Nov 23, 2022

Good idea, I'll add some coverage. I did test them in the demos

* master:
  docs: update component READMEs (#5810)
  ci(chromatic): use diffThreshold value saved in secrets (#5806)
  1.0.0-next.642
  fix(combobox): 5540 - handle focus (#5774)
  1.0.0-next.641
  fix(tree-item): Allow space and enter key events when selectionMode is "none" (#5800)
  Update CONTRIBUTING.md
  1.0.0-next.640
  fix(input-date-picker): display updated valueAsDate in the two range inputs (#5758)
  docs(tree): Update interface for selectionMode. (#5801)
  docs: update component READMEs (#5804)
@benelan benelan added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Nov 28, 2022
@benelan benelan merged commit feb4fce into master Nov 28, 2022
@benelan benelan deleted the benelan/4079-add-native-input-attributes branch November 28, 2022 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants