Skip to content

Commit

Permalink
fix(searchbar): autocapitalize is initialized correctly (#29197)
Browse files Browse the repository at this point in the history
Issue number: resolves #29193

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

In an attempt to preserve backwards compatibility when adding
`autocapitalize` to Searchbar, we used `!` to indicate that the prop was
never undefined. The `autocapitalize` on `HTMLElement` expects this
value to be a string and never undefined.

For the purposes of the property on Searchbar, setting this prop to one
of the accepted values would constitute a breaking change because it
would override the default browser behavior (which we previously relied
upon). As a result, we used `!` to not set a default prop but inform
TypeScript that this prop is always defined. This unintentionally made
it so developers needed to define the `autocapitalize` property every
time which is not what we want.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- `autocapitalize` now defaults to the `'default'` keyword. This is an
internal keyword that is used to tell Ionic to **not** set the
`autocapitalize` attribute on the inner `input` element and instead rely
on the default browser behavior. This satisfies the `HTMLElement`
requirement that `autocapitalize` is never undefined. In Ionic 8 this
`'default'` value will be replaced with `'off'`.

[Typescript currently sets the `HTMLElement` `autocapitalize` type to
`string`](https://github.com/microsoft/TypeScript/blob/65812bf3ec3b9208141ef46e43d146a2eef88ae5/src/lib/dom.generated.d.ts#L10087)
which is why we can add a `'default'` keyword here. There is some risk
that if these type definitions change in the future that applications
may encounter errors. However, changing this on the TypeScript side
would itself be a breaking change. Additionally, we are moving away from
`'default'` in Ionic 8, so I think this is an acceptable amount of risk.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->


Dev build: `7.8.2-dev.11711027016.13b2a920`

Tested in a React starter app with Searchbar, and I verified this fix
works.
  • Loading branch information
liamdebeasi authored Mar 21, 2024
1 parent d1253c0 commit 8ad66c0
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 10 deletions.
2 changes: 1 addition & 1 deletion core/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ ion-row,shadow

ion-searchbar,scoped
ion-searchbar,prop,animated,boolean,false,false,false
ion-searchbar,prop,autocapitalize,string,undefined,true,false
ion-searchbar,prop,autocapitalize,string,'default',false,false
ion-searchbar,prop,autocomplete,"name" | "email" | "tel" | "url" | "on" | "off" | "honorific-prefix" | "given-name" | "additional-name" | "family-name" | "honorific-suffix" | "nickname" | "username" | "new-password" | "current-password" | "one-time-code" | "organization-title" | "organization" | "street-address" | "address-line1" | "address-line2" | "address-line3" | "address-level4" | "address-level3" | "address-level2" | "address-level1" | "country" | "country-name" | "postal-code" | "cc-name" | "cc-given-name" | "cc-additional-name" | "cc-family-name" | "cc-number" | "cc-exp" | "cc-exp-month" | "cc-exp-year" | "cc-csc" | "cc-type" | "transaction-currency" | "transaction-amount" | "language" | "bday" | "bday-day" | "bday-month" | "bday-year" | "sex" | "tel-country-code" | "tel-national" | "tel-area-code" | "tel-local" | "tel-extension" | "impp" | "photo",'off',false,false
ion-searchbar,prop,autocorrect,"off" | "on",'off',false,false
ion-searchbar,prop,cancelButtonIcon,string,config.get('backButtonIcon', arrowBackSharp) as string,false,false
Expand Down
2 changes: 1 addition & 1 deletion core/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7295,7 +7295,7 @@ declare namespace LocalJSX {
/**
* Indicates whether and how the text value should be automatically capitalized as it is entered/edited by the user. Available options: `"off"`, `"none"`, `"on"`, `"sentences"`, `"words"`, `"characters"`.
*/
"autocapitalize": string;
"autocapitalize"?: string;
/**
* Set the input's autocomplete property.
*/
Expand Down
15 changes: 7 additions & 8 deletions core/src/components/searchbar/searchbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,19 @@ export class Searchbar implements ComponentInterface {
* and disabled by default on Android
* for Searchbar. The autocapitalize type on HTMLElement
* requires that it be a string and never undefined.
* However, setting it to a string value would be a breaking change
* in behavior, so we use "!" to tell TypeScript that this property
* is always defined so we can rely on the browser defaults. Browsers
* will automatically set a default value if the developer does not set one.
* However, setting it to one of the accepted values would be a breaking change
* in behavior, so we use the internal "default" keyword to ensure
* we use the default browser behavior for now.
*
* In the future, this property will default to "off" to align with
* Input and Textarea, and the "!" will not be needed.
* Input and Textarea, and the internal "default" keyword will not be needed.
*/

/**
* Indicates whether and how the text value should be automatically capitalized as it is entered/edited by the user.
* Available options: `"off"`, `"none"`, `"on"`, `"sentences"`, `"words"`, `"characters"`.
*/
@Prop() autocapitalize!: string;
@Prop() autocapitalize: string = 'default';

/**
* Set the input's autocomplete property.
Expand Down Expand Up @@ -621,7 +620,7 @@ export class Searchbar implements ComponentInterface {
}

render() {
const { cancelButtonText } = this;
const { cancelButtonText, autocapitalize } = this;
const animated = this.animated && config.getBoolean('animated', true);
const mode = getIonMode(this);
const clearIcon = this.clearIcon || (mode === 'ios' ? closeCircle : closeSharp);
Expand Down Expand Up @@ -683,7 +682,7 @@ export class Searchbar implements ComponentInterface {
placeholder={this.placeholder}
type={this.type}
value={this.getValue()}
autoCapitalize={this.autocapitalize}
autoCapitalize={autocapitalize === 'default' ? undefined : autocapitalize}
autoComplete={this.autocomplete}
autoCorrect={this.autocorrect}
spellcheck={this.spellcheck}
Expand Down

0 comments on commit 8ad66c0

Please sign in to comment.