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

AS-407 Ember 4 change | Refactor ariaInvalid property #79

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

marianoggf
Copy link
Contributor

There are instances where we set ariaInvalid. For example:

<PolarisTextField
  @ariaInvalid={{this.someMethod}}
/>

However, since it is a boolean computed property, we cannot use this approach in Ember 4 anymore. This PR solves this issue by adding a getter and a setter for ariaInvalid.

@marianoggf marianoggf self-assigned this Mar 8, 2024
@marianoggf marianoggf requested a review from a team as a code owner March 8, 2024 19:34
@marianoggf marianoggf requested review from nahrivera7 and removed request for a team March 8, 2024 19:34
Copy link

swarmia bot commented Mar 8, 2024

✅  Linked to Task AS-407 · Upgrade to latest Ember 4.x
➡️  Part of Epic AS-405 · [smile-admin] Upgrade to latest Ember 4.x

@marianoggf marianoggf merged commit 8eeefa1 into main Mar 8, 2024
9 checks passed
@marianoggf marianoggf deleted the as-407-ember-4-change-refactor-ariaInvalid branch March 8, 2024 20:27
Comment on lines +358 to +365
@computed('error', '_ariaInvalid')
get ariaInvalid() {
return this._ariaInvalid ?? !!this.error;
}

set ariaInvalid(value) {
this.set('_ariaInvalid', value);
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch fixing this.

Small nit - I think we chatted about this in another PR - but wondering if this simpler form works
@computed('error')
get ariaInvalid() {
return !!this.error;
}
set ariaInvalid(value) {
return value;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants