fix(select): remove item focus style when there are no selected items only at ionic mode#30750
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thetaPC
left a comment
There was a problem hiding this comment.
Few things as well:
- Checks are failing. Please have those passing before asking for a review.
- This is causing a regression. Notice how there's a border radius when opening a select popover:
next |
ROU-12235 |
|---|---|
| https://github.com/user-attachments/assets/a156e4da-2fbc-472b-9b33-b9aaa85db78c | https://github.com/user-attachments/assets/ff9d844d-ddbf-42b4-baf0-632ac8cf6053 |
I haven't checked the other interfaces but I assume that they have the same issue. Please check.
- What's the ticket to add in the work for "This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling."? Because the options are no longer showing the focus rings when navigating with keyboard.
...cordion.e2e.ts-snapshots/accordion-states-focused-ionic-md-ltr-light-Mobile-Chrome-linux.png
Show resolved
Hide resolved
...n.e2e.ts-snapshots/accordion-states-inset-focused-ionic-md-ltr-light-Mobile-Chrome-linux.png
Show resolved
Hide resolved
...est/states/item.e2e.ts-snapshots/item-states-diff-ionic-md-ltr-light-Mobile-Chrome-linux.png
Show resolved
Hide resolved
...ts-snapshots/toolbar-basic-nested-slotted-images-ionic-md-ltr-light-Mobile-Firefox-linux.png
Show resolved
Hide resolved
Basically we must add this behaviour just when keyboard navigation is used to navigate through the elements, before this implementation it was defined on top of tap navigation which causes this visual issue, now focus style is missing when there are no pre-selected option when keyboard navigation is in use. |
thetaPC
left a comment
There was a problem hiding this comment.
Just realized that we should add a test under select-modal to capture the focus states when an option is selected and when it's not.
...lays/datetime-button.e2e.ts-snapshots/datetime-overlay-modal-ios-ltr-Mobile-Safari-linux.png
Show resolved
Hide resolved
...bar.e2e.ts-snapshots/toolbar-basic-slotted-images-ionic-md-ltr-light-Mobile-Chrome-linux.png
Show resolved
Hide resolved
@joselrio I understand that the behavior needs to be added for keyboard. What's the Jira ticket that implements it? |
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
I agree, but let's do it under the context of the task I mentioned before since focus should be specially tackled under that context ;) |
Task do not exist yet, but it will be created since we already aligned with @jessicamendesOS and @gnbm about it. |
Issue number: resolves #
This PR introduces improvements to the visual focus styling of Ionic items when inside a select modal.
What is the current behavior?
--background-focusedand--background-focused-opacitywere missing fromitem.ionic.scss, which resulted in the native default outline focus style being applied to focused items.What is the new behavior?
NOTE
Does this introduce a breaking change?
Other information