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

Input: Add autocapitalize, autocorrect, enterkeyhint, and spellcheck properties #2351

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

austinoneil
Copy link
Contributor

@austinoneil austinoneil commented Mar 25, 2024

Description

Add autocapitalize, autocorrect, enterkeyhint, and spellcheck properties to input

References #2347

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

Added unit tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for moduswebcomponents ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1a1de52
🔍 Latest deploy log https://app.netlify.com/sites/moduswebcomponents/deploys/660bc196d8c50d00081fecb4
😎 Deploy Preview https://deploy-preview-2351--moduswebcomponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 26 (🔴 down 1 from production)
Accessibility: 98 (no change from production)
Best Practices: 92 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@austinoneil austinoneil changed the title Add autocapitalize, autocorrect, enterkeyhing, and spellcheck properties to input Add autocapitalize, autocorrect, enterkeyhint, and spellcheck properties to input Mar 25, 2024
@coliff coliff self-requested a review March 25, 2024 23:39
@coliff coliff changed the title Add autocapitalize, autocorrect, enterkeyhint, and spellcheck properties to input Input: Add autocapitalize, autocorrect, enterkeyhint, and spellcheck properties Mar 26, 2024
Copy link
Member

@coliff coliff left a comment

Choose a reason for hiding this comment

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

Nice! looks good.. but we need to have the additional controls to the demo canvas:
https://deploy-preview-2351--moduswebcomponents.netlify.app/?path=/story/user-inputs-text-input--default
(autocapitalize, autocorrect, enterkeyhint, and spellcheck options should be available here)

I don't think we need demos of them on the Docs page, can you please remove?

@austinoneil
Copy link
Contributor Author

@coliff done.

@coliff
Copy link
Member

coliff commented Mar 26, 2024

  • autocapitalize is appearing twice
  • spellcheck is missing
  • changing the values of the autocapitalize, autocorrect and enterkeyhint does not change the input at all. See screenshot:
    image
    We expect the input with id="mwc_id_0_text_input" here to have enterkeyhint="done" autocapitalize="sentences" autocorrect="off"

@austinoneil
Copy link
Contributor Author

Oops. Fixed.

@coliff
Copy link
Member

coliff commented Mar 27, 2024

thanks for work on this so far - still a few more things:

  • autocapitalize by default should not be set at all. autocapitalize attribute should not be set on the mwc_id_9_text_input in the canvas example. (autocapitalize is the default on mobile devices usually anyway). So the dropdown in the Controls as standard should be 'Choose option...'
  • autocorrect by default should not be set at all. autocorrect attribute should not be set on the mwc_id_9_text_input in the canvas example. (autocorrect is the default on mobile devices usually anyway). So the dropdown in the Controls as standard should be 'Choose option...'
  • enterkeyhint should not have an option pre-selected - so have a ('Choose option...') selected.
  • spellcheck should be a dropdown with true or false options - but by default nothing selected ('Choose option...').

@austinoneil
Copy link
Contributor Author

I made the changes (except for spellcheck, as that is an attribute that is true if it's included, much like disabled).

coliff
coliff previously approved these changes Mar 28, 2024
@coliff coliff merged commit 76eb482 into trimble-oss:main Apr 2, 2024
7 of 8 checks passed
austinoneil pushed a commit to austinoneil/modus-web-components that referenced this pull request Apr 3, 2024
austinoneil added a commit to austinoneil/modus-web-components that referenced this pull request Apr 4, 2024
…lcheck `properties (trimble-oss#2351)

* Add autocapitalize, autocorrect, enterkeyhing, and spellcheck properties to input

* Update modus-text-input-storybook-docs.mdx

---------

Co-authored-by: Austin O'Neil <austinoneil@Austins-MacBook-Pro-2.local>
Co-authored-by: coliff <christianoliff@pm.me>
@kuhnboy
Copy link

kuhnboy commented Apr 5, 2024

@austinoneil Why make autocorrect: boolean | 'off' | 'on';

Why not stick to just an enumeration or just a boolean?

@austinoneil
Copy link
Contributor Author

@kuhnboy When I as a developer see autocorrect as an attribute, my instinct tells me that it is a boolean input; however, the autocorrect attribute on input takes "on" and "off" as values. I included both because doing so makes the library easier to use without creating any obvious problems.

@austinoneil
Copy link
Contributor Author

@kuhnboy That said, it does look like this was causing issues with autocapitalize in react (see #2412), so if needed, we can just use "on" and "off" (or just boolean)

coliff added a commit that referenced this pull request Jun 11, 2024
* Add `modus-textarea-input` component.

* add autocorrect, autocapitalize, enterkeyhing, spellcheck to textarea reflecting #2351

* add documentation

* fix merge conflicts (I think)

* removed resizability for textarea

* remove angular changes from pull request

* Update package.json

* Delete angular-workspace/ng14/projects/trimble-oss/modus-angular-components/src/lib/stencil-generated/angular-component-lib/utils.ts

* Delete angular-workspace/ng14/package-lock.json

* manually removed angular autogenerated stuff

* manually removed react autogenerated files

* add ng14 lock back

* revert icon readme chages

---------

Co-authored-by: Austin O'Neil <austinoneil@Austins-MacBook-Pro-2.local>
Co-authored-by: Christian Oliff <christianoliff@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants