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

bug: additional properties for searchbar #27606

Closed
3 tasks done
AmitMY opened this issue Jun 6, 2023 · 15 comments
Closed
3 tasks done

bug: additional properties for searchbar #27606

AmitMY opened this issue Jun 6, 2023 · 15 comments
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement

Comments

@AmitMY
Copy link
Contributor

AmitMY commented Jun 6, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

The input inside ion-searchbar does not support all possible attributes.

<ion-searchbar animated="true" search-icon="code-working-outline" 
aria-label="Spoken Language Text" placeholder="Enter Text" autocomplete="off" autocorrect="off" spellcheck="false" 
autofocus dir="auto" aria-autocomplete="list" autocapitalize="off" data-gramm_editor="false" lang="en" maxlength="100">

Results in:

<input 
aria-label="search text" placeholder="Enter Text" autocomplete="off" autocorrect="off" spellcheck="false">

Expected Behavior

Should inherit attributes from ion-searchbar.

Input should also have

autofocus dir="auto" aria-autocomplete="list" autocapitalize="off" data-gramm_editor="false" lang="en" maxlength="100"

Steps to Reproduce

Create an ion-searchbar:

<ion-searchbar animated="true" search-icon="code-working-outline" 
aria-label="Spoken Language Text" placeholder="Enter Text" autocomplete="off" autocorrect="off" spellcheck="false" 
autofocus dir="auto" aria-autocomplete="list" autocapitalize="off" data-gramm_editor="false" lang="en" maxlength="100">

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 7.1.1

Utility:

cordova-res : 0.15.4
native-run : 1.7.2

System:

NodeJS : v18.15.0
npm : 9.6.7
OS : macOS Unknown

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Jun 6, 2023
@liamdebeasi
Copy link
Contributor

Thanks for the report. Can you clarify which attributes you are expecting ion-searchbar to inherit?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jun 6, 2023
@ionitron-bot ionitron-bot bot removed the triage label Jun 6, 2023
@AmitMY
Copy link
Contributor Author

AmitMY commented Jun 6, 2023

Whatever a normal input supports, namely in this case:

  • autofocus
  • dir="auto"
  • aria-autocomplete="list"
  • autocapitalize="off"
  • lang="en"
  • maxlength="100"

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jun 6, 2023
@colabottles
Copy link

I'd advise against using aria-autocomplete due to the lack of screen reader support in browsers. Unless you use the value of none, you will have no mobile screen reader support when using the "list" value on the aria-autocomplete attribute. More up-to-date information can be found here.

Also, when using aria-autocomplete you're looking at a lot of work with the nuances to make it accessible, but you can do it. Just right now, screen reader support is shaky.

@AmitMY
Copy link
Contributor Author

AmitMY commented Jul 27, 2023

Thanks @colabottles
I still think it should inherit Whatever a normal input supports, but I understand the accessibility issues with this particular attribute, and will find an alternative

@colabottles
Copy link

@AmitMY You'll be hard-pressed to find an alternative. Until screen reader support improves, this will remain an issue and using an aria attribute with no support is counter-intuitive. It can always be added in when the support is there.

@liamdebeasi
Copy link
Contributor

Hey @AmitMY,

I spoke with the team, and we are interested in learning more about the use cases for the following requested attributes:

  • autofocus
  • dir
  • autocapitalize
  • lang
  • maxlength

We are open to adding new properties to ion-searchbar, but we need to understand the use case for each request before we can proceed.

As @colabottles noted, aria-autocomplete does not have great screen reader support, so we do not have plans to support this at the moment. However, we'd be happy to consider a future request if support improves.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jul 27, 2023
@ionitron-bot ionitron-bot bot removed the triage label Jul 27, 2023
@AmitMY
Copy link
Contributor Author

AmitMY commented Jul 28, 2023

Hi @liamdebeasi
I feel like these are just standard input attributes, and that you should just try to inherit everything, rather than a case-by-case basis.

For your question:

  • autofocus for when my app is opened, it should actually focus on the search bar, which is in the footer, similar to how it is on iOS safari with the URL bar (which is a search bar)
  • dir is for me to allow both ltr and rtl writing. I want to do dir="auto" so that you can write hebrew (my native language) and that it shows correctly. right now, it doesn't.
  • lang is for autocorrect. My app is a translation app, and thus you can select your input from 100+ languages. I want the phone to know which language it is autocorrecting for.
  • autocapitalize gives me cleaner inputs to work with, but is not "mandatory"
  • maxlength is used to prevent too long of inputs, relevant to both client side models running to identify the language, or to translate it, or to server models, or to database entries and logs.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jul 28, 2023
@colabottles
Copy link

@AmitMY With all due respect, implementing aria without optimal screen reader support is really discouraged because if aria is implemented with shoddy support, the ramifications to screen reader users in screen readers not supported is they won't get the full accessible experience and the screen reader won't read the content as it is supposed to be read to them. This is taking a giant step back from the work the Ionic team is taking.

Inheriting everything is a goal, but in the short term, I'd rather see the team implement things correctly and accessibly for the people with disabilities I advocate for. Leaving disable users behind because you would like to see everything inherited does not make sense and is exclusionary. You're excluding those users from the full experience they have a right to.

@AmitMY
Copy link
Contributor Author

AmitMY commented Jul 28, 2023

@colabottles This really is not the debate here... The decision of using or not using an attribute should be on the developer.

I argue for the inheritance of all attributes, since this is how inputs work. HOWEVER, there can be made exclusions, such as aria-autocomplete="list" that would not be supported, with a good reason.

I just don't think that every single attribute requires justification or a use case. Inherit all by default, exclude things that shouldn't be there. (I go with include all at first, exclude some later, rather than exclude all and include only when makes sense)

I'd even go one more step and say that it should not be inherited by normal ion-input, for the reasons you mentioned.

@colabottles
Copy link

This is my final input here. You can go with what you will go with as your thought process @AmitMY, but I stand with Ionic's decision to wait on implementation of any ARIA that has terrible support for assistive technology.

This really is not the debate here...

Yes it is, you're campaigning for a product to implement something that is inaccessible. So it most certainly is the debate here and my job as an Accessibility Advocate is to support the disabled community and disabled developers, then it is my job to educate the developer first and find a resolution to something they want to implement that is inaccessible so that it can be accessible.

The decision of using or not using an attribute should be on the developer.

I agree. Do you know the impact of using ARIA that is inaccessible and creates conflicts with assisted technologies? If you do not know, again, it is my job to educate developers on how to make accessible things.

I just don't think that every single attribute requires justification or a use case.

They certainly do when they create barriers for people with disabilities and make an accessible framework inaccessible, which is what adding aria-autocomplete will do because of the screen reader support.

In 25 years, I have never NOT justified use of ARIA when it was debated as needed. It's not unless you have the support of the browser and the assistive technology in my space (which is to advocate for people with disabilities). Especially when it must convey the "none" value and not "list" as you would like to see it in your original reply.

aria-autocomplete may have the browser support, but it does not have the screen reader support to justify including it which you go back and forth with wanting aria-autocomplete="list" and then it appears you don't. But you don't inherit everything when it comes to ARIA, that's not how that works.

Why make more work for not only yourself, but for developers who may know the implications of using aria-autocomplete-"list" and the impact that has on accessibility. The disabled community has a lot of developers that are well aware of this. Not everyone that develops/is a developer is an abled individual.

@sean-perkins
Copy link
Contributor

Hello @AmitMY can you share a screenshot or recording of the issues you are experiencing with using dir on ion-searchbar?

Here's a small reproduction case: https://stackblitz.com/edit/angular-htjrcc?file=src%2Fapp%2Fexample.component.html

dir should cascade to descendent elements natively.

@sean-perkins sean-perkins added the needs: reply the issue needs a response from the user label Aug 15, 2023
@ionitron-bot ionitron-bot bot removed the triage label Aug 15, 2023
@AmitMY
Copy link
Contributor Author

AmitMY commented Aug 15, 2023

Hi @sean-perkins
My problem is with the "auto" one.
Just write "עברית" in there, and see. (means Hebrew, in Hebrew)
image

If I indeed force the input to be dir="auto" (inspect element), I get:
image

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Aug 15, 2023
@sean-perkins
Copy link
Contributor

Thanks for following up! I can confirm the same thing, thanks for the callout to use Hebrew text with the auto direction.

This seems to affect ion-searchbar, ion-textarea and ion-input: https://stackblitz.com/edit/angular-htjrcc-vlfmh6?file=src%2Fapp%2Fexample.component.html

Manually modifying the inner control to assign the dir attribute directly results in the correct text direction.

I need to discuss with the team how we want to track this work. We don't want to inherit all attributes, but there are some problematic attributes that should inherit that currently do not. We should be consistent to applying these attributes to similar text controls that have the same challenges.

@liamdebeasi liamdebeasi changed the title bug: ion-searchbar input should inherit all attributes bug: additional properties for searchbar Sep 19, 2023
@liamdebeasi liamdebeasi added the type: feature request a new feature, enhancement, or improvement label Sep 19, 2023
@liamdebeasi liamdebeasi added the package: core @ionic/core package label Sep 19, 2023
liamdebeasi added a commit that referenced this issue Mar 1, 2024
…are inherited to native input (#29098)

Issue number: resolves #27606

---------

<!-- 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. -->

Certain attributes are not be inherited to the inner searchbar.
Developers need control over these attributes to provide important
context to users for things like language and text direction.
Additionally, being able to control things like autocapitalize,
maxlength, and minlength can help improve the user experience by a)
guiding what should be entered into an input and b) removing
autocapitalize where it's not appropriate.

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

- Added autocapitalize, maxlength, and minlength properties
- lang and dir are global attributes, so adding them as properties will
cause issues. However, developers can still set them as attributes and
they will be inherited to the native `input` element. We also watch them
so any changes to the attributes are also inherited to the native
`input`.

## 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. -->

Note: We expanded the scope of this work to also include input and
textarea, and this work will be handled separately. However, the
original request was only for searchbar so that's why I associated this
PR with the linked issue.

Dev build: `7.7.3-dev.11709159644.114cd8b1`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #29098, and a fix will be available in an upcoming release of Ionic Framework.

A couple things to note:

  1. dir and lang will be attributes that get inherited to the inner input instead of properties, so they won't show up in the "Properties" section of the searchbar docs. We did this to avoid colliding with the global attributes of the same name (doing so can cause issues). However, this should not impact usage of this feature -- just something to be aware of if you're wondering where they are in the docs.
  2. We still plan to address the issues in input and textarea, and we are tracking this internally.

The original issue focused on the searchbar which we've addressed, so I am going to close this.

Copy link

ionitron-bot bot commented Mar 31, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Mar 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

No branches or pull requests

4 participants