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

fix(range, select): prefer labels passed by developer #29145

Merged
merged 12 commits into from
Mar 14, 2024
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Mar 12, 2024

Issue number: Internal


What is the current behavior?

With the removal of the legacy form syntax the team wanted to understand the value that getAriaLabel provides to determine if we still need this.

This utility was helpful for scenarios where developers provided their own label outside of the component. For example:

<div id="foo">Checkbox label</div>
<ion-checkbox aria-labelledby="foo"></ion-checkbox>

The challenge with this is the component isn't actually using aria-labelledby. We're grabbing the text content of #foo and setting it as an aria-label on the checkbox, so the flexibility here is fairly limited. The same goes for developers who want to use a <label> element and associated it with a component.

With the introduction of the modern form syntax labels should be provided directly on the form control (either via a visible text label or an aria-label). Additionally, this functionality was already removed from several components including checkbox. There has not been any community feedback for ~1.5 years which makes me think the patterns that getAriaLabel accounts for are not being used frequently.

Given that support for these patterns are a) do not seem to be used and b) are implemented inconsistently since only select and range support them, I propose we remove getAriaLabel altogether.

What is the new behavior?

  • Ionic Framework will no longer automatically find external labels and apply them for developers. Developers can continue to achieve this behavior by manually applying an aria-label or visible text on the form control.
  • Updated component guide to note how we handle form control labels now
  • Removing this code uncovered an inconsistency in main where aria labels were effectively ignored when both visible text and an aria label is provided on range and select. I fixed this bug too.

Does this introduce a breaking change?

  • Yes
  • No

Rather than make this a new breaking change I've decided to update the select/range breaking changes to note this since this change is part of the modern form syntax updates.

Other information

@github-actions github-actions bot added the package: core @ionic/core package label Mar 12, 2024
@liamdebeasi liamdebeasi marked this pull request as ready for review March 13, 2024 18:37
@liamdebeasi liamdebeasi requested a review from a team as a code owner March 13, 2024 18:37
@liamdebeasi liamdebeasi requested review from brandyscarney and removed request for a team March 13, 2024 18:37
@liamdebeasi liamdebeasi changed the title refactor(range, select): prefer labels passed by developer fix(range, select): prefer labels passed by developer Mar 14, 2024
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Works great! 👍

@liamdebeasi liamdebeasi merged commit 56014cf into feature-8.0 Mar 14, 2024
44 checks passed
@liamdebeasi liamdebeasi deleted the FW-5969 branch March 14, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants