fix(rating): adds focus outline on click #7341

anveshmekala · 2023-07-19T17:37:48Z

Related Issue: #4633 , #6642

Summary

This PR will add focus outline for calcite-rating component on click.

github-actions · 2023-07-28T01:43:29Z

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

driskull

👍 LGTM. Lets get one other review before merging.

jcfranco

Awesome!

Had a few comments, but this LGTM! 🚀

Risk-wise, I think we should wait after the upcoming maintenance release is done before landing this one.

packages/calcite-components/src/components/rating/rating.e2e.ts

packages/calcite-components/src/components/rating/rating.tsx

jcfranco · 2023-08-17T16:48:48Z

packages/calcite-components/src/components/rating/rating.tsx

                    htmlFor={id}
+                    onClick={this.handleLabelClick}


Interesting. I thought this was a browser bug, but it seems to be working as intended (see Pointer Event: Suppress compatibility mouse events on click test and results. Can you add a comment about this for posterity?

jcfranco · 2023-08-17T16:50:49Z

packages/calcite-components/src/components/rating/rating.e2e.ts

      expect(await element.getProperty("value")).toBe(0);
      expect(changeEvent).toHaveReceivedEventTimes(1);
+      expect(hoveredEl.length).toBe(3);
+      expect(await isElementFocused(page, `[for=${focusedElId}]`, { shadowed: true })).toBe(true);


Not critical, but is it possible to query using the star class via label or label attributes? This would make the test less brittle in case the implementation changes.

packages/calcite-components/src/components/rating/rating.tsx

jcfranco · 2023-08-17T16:52:43Z

packages/calcite-components/src/components/rating/rating.tsx

                    onPointerDown={this.handleLabelPointerDown}
                    onPointerOver={this.handleLabelPointerOver}
+                    tabIndex={tabIndex}


Does making label have any impact on how screen readers interpret the value?

the screen reader experience has no impact in VoiceOver. cc @geospatialem

I do see a lot of intermediate states with VoiceOver where visually hidden area are being focused when user is navigating using voiceover commands + rightArrow. we need to add aria-hidden attribute to remove the visually hidden elements from accessibility tree. This can be done in a different PR though, wanted to confirm similar behavior with NVDA/JAWS before diving in.

One addition could be announcing to AT user when user reset the selection ( not available in native behavior).

Not following the logic above, is this to remove the label?

jcfranco · 2023-08-17T16:59:16Z

fix(rating): adds focus outline on click

One more thing, should the title reflect that focus is added for both pointer and keyboard usage? It reads as only covering the pointer case.

anveshmekala · 2023-08-17T23:30:10Z

fix(rating): adds focus outline on click

One more thing, should the title reflect that focus is added for both pointer and keyboard usage? It reads as only covering the pointer case.

Since we marked this as fix i think its safe to mention click only and we attached the issue too.

geospatialem

Changes look good for mouse users. Also verified with JAWS, just for testing sake, and we're good to go. 👍🏻

geospatialem · 2023-08-25T18:40:39Z

packages/calcite-components/src/components/rating/rating.tsx

                    onPointerDown={this.handleLabelPointerDown}
                    onPointerOver={this.handleLabelPointerOver}
+                    tabIndex={tabIndex}


Not following the logic above, is this to remove the label?

**Related Issue:** #4633 , #6642 ## Summary This PR will add focus outline for `calcite-rating` component on click.

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.6.1...@esri/calcite-components@1.7.0) (2023-09-01) ### Features * **action-bar, action-pad, action-group:** Add label properties for group context ([#7415](#7415)) ([b34f36d](b34f36d)) * **combobox:** Add single-persist selection mode ([#7583](#7583)) ([dab06a3](dab06a3)) * **flow:** Add support for custom flow-item elements ([#7608](#7608)) ([197adfe](197adfe)) * **input-date-picker:** Normalize year to current century for user typed values only ([#7638](#7638)) ([a1db718](a1db718)) * **input-number:** Add integer property ([#7646](#7646)) ([cd66a6d](cd66a6d)) * **input-time-picker:** Support fractional seconds ([#7532](#7532)) ([c2bf34b](c2bf34b)) * **meter:** Add Meter component ([#7401](#7401)) ([47163ed](47163ed)) * **sheet:** Add Sheet component ([#7561](#7561)) ([f12a393](f12a393)) * **sheet:** Update default widths ([#7617](#7617)) ([47d2c0b](47d2c0b)) * **shell:** Add "Sheets" Slot ([#7579](#7579)) ([e798765](e798765)) * **table:** Add Table and related components ([#7607](#7607)) ([b067e72](b067e72)) ### Bug Fixes * **accordion, accordion-item:** Improve a11y ([#7560](#7560)) ([b5170b6](b5170b6)) * Add drag styles for improved UX ([#7644](#7644)) ([afbb764](afbb764)) * **block, block-section:** Improve a11y ([#7557](#7557)) ([1f44f6b](1f44f6b)) * **chip-group:** Add existence checks ([#7586](#7586)) ([5ca64f1](5ca64f1)) * **color-picker:** Update value when alphaChannel is toggled ([#7563](#7563)) ([1f753dd](1f753dd)) * **combobox:** Prevent deselecting items via keyboard in single-persist mode ([#7634](#7634)) ([4f5f8b0](4f5f8b0)) * **combobox:** Update combobox height to follow design spec ([#7558](#7558)) ([ec08845](ec08845)) * **date-picker:** Set start of week to monday in zh-CN ([#7578](#7578)) ([7e385cb](7e385cb)) * **dropdown:** Prevents navigating dropdown items with Tab key ([#7527](#7527)) ([3ea658d](3ea658d)) * Ensure label only focuses the first labelable child ([#7553](#7553)) ([426159c](426159c)) * **flow:** Catch error when beforeBack promise is rejected ([#7601](#7601)) ([cb70671](cb70671)) * **input-date-picker, input-time-picker:** Do not show dropdown affordance when read-only ([#7559](#7559)) ([5a3f19c](5a3f19c)) * **input, input-number:** Correctly sanitize numbers when pasting string with 'e' ([#7648](#7648)) ([b8bc11c](b8bc11c)) * **list, sortable-list, value-list:** Emit calciteListOrderChange when dragging between lists ([#7614](#7614)) ([4653581](4653581)) * **list:** Fixes dragging nested list items ([#7555](#7555)) ([c25f7b3](c25f7b3)) * **list:** Stop emitting calciteListChange when a list-item is disabled or closed. ([#7624](#7624)) ([7008463](7008463)) * **loader:** Tweak loading animations to work in Safari ([#7564](#7564)) ([2103654](2103654)) * **modal:** Catch error when beforeClose promise is rejected ([#7600](#7600)) ([70405d0](70405d0)) * **modal:** Handle removal of open attribute and prevent multiple beforeClose calls ([#7470](#7470)) ([f31588f](f31588f)) * **rating:** Adds focus outline on click ([#7341](#7341)) ([af30073](af30073)) * **segmented-control:** Refresh items when added dynamically ([#7567](#7567)) ([2e36eb3](2e36eb3)) * **split-button:** Update divider and borders to follow design spec ([#7568](#7568)) ([8df59ab](8df59ab)) * **tree-item:** Move focus outline to item label area ([#7581](#7581)) ([1327cef](1327cef)) * **tree-item:** Updates state when selection changes programmatically for `selection-mode='ancestors'` ([#7572](#7572)) ([40758c5](40758c5)) * **tree:** Improve keyboard navigation ([#7618](#7618)) ([826a5cb](826a5cb)) </details> <details><summary>@esri/calcite-components-react: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.6.1...@esri/calcite-components-react@1.7.0) (2023-09-01) ### Bug Fixes * Make sure components are defined in environments like in codesandbox ([#7632](#7632)) ([7005cce](7005cce)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.7.0-next.22 to ^1.7.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

anveshmekala added 4 commits July 14, 2023 17:58

fix(combobox): announces all selected items to AT users

5f11d98

Merge branch 'main' of https://github.com/Esri/calcite-components

08e7208

Merge branch 'main' of https://github.com/Esri/calcite-components

eb62755

feat(rating,switch): updates focus outline

feaadb4

github-actions bot added the enhancement Issues tied to a new feature or request. label Jul 19, 2023

github-actions bot assigned anveshmekala Jul 19, 2023

anveshmekala added 4 commits July 19, 2023 12:40

revert combobx changes

2e6fd9e

fix e2e errors

556d02b

Merge branch 'main' into anveshmekala/4633-fix-hitbox-focus

6b59555

add key for label element

fb1b9af

github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jul 28, 2023

anveshmekala changed the title ~~feat(rating,switch): updates focus outline~~ feat(rating): updates focus outline Aug 1, 2023

anveshmekala added 13 commits August 4, 2023 15:27

refactor

d393ea0

Merge branch 'main' into anveshmekala/4633-fix-hitbox-focus

d74e04d

revert switch component changes

4e6f24d

prevent focus from shifting to input on label click

13cc13e

fix tests

d9265d1

refactor

6ffdff5

remove hasFocus state property

314330e

remove focusValue state property

c5fd09f

Merge branch 'main' into anveshmekala/4633-fix-hitbox-focus

a132136

fix setFocus()

ebc255d

cleanup

a05fc2d

fix focus tests

e467dff

add focus test for chromatic

dd228b1

anveshmekala removed the Stale Issues or pull requests that have not had recent activity. label Aug 9, 2023

anveshmekala marked this pull request as ready for review August 9, 2023 17:03

anveshmekala requested a review from a team as a code owner August 9, 2023 17:03

anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 9, 2023

feedback changes

3df8cc1

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Aug 9, 2023

anveshmekala requested a review from driskull August 9, 2023 21:28

driskull approved these changes Aug 9, 2023

View reviewed changes

anveshmekala requested a review from jcfranco August 10, 2023 14:42

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 15, 2023

jcfranco approved these changes Aug 17, 2023

View reviewed changes

feedback changes

efc577b

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 18, 2023

add comments

fa9c5d9

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 18, 2023

Merge branch 'main' into anveshmekala/4633-fix-hitbox-focus

3ca7674

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 25, 2023

geospatialem reviewed Aug 25, 2023

View reviewed changes

update selector in tests

77784d2

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 26, 2023

anveshmekala merged commit af30073 into main Aug 26, 2023

anveshmekala deleted the anveshmekala/4633-fix-hitbox-focus branch August 26, 2023 02:30

github-actions bot added this to the 2023 August Priorities milestone Aug 26, 2023

anveshmekala mentioned this pull request Aug 26, 2023

Hitbox focus on components #4633

Closed

7 tasks

calcite-admin mentioned this pull request Aug 25, 2023

chore: release main #7571

Merged

Elijbet pushed a commit that referenced this pull request Aug 30, 2023

fix(rating): adds focus outline on click (#7341)

6ce661f

**Related Issue:** #4633 , #6642 ## Summary This PR will add focus outline for `calcite-rating` component on click.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rating): adds focus outline on click #7341

fix(rating): adds focus outline on click #7341

anveshmekala commented Jul 19, 2023 •

edited

Loading

github-actions bot commented Jul 28, 2023

driskull left a comment

jcfranco left a comment

jcfranco Aug 17, 2023

jcfranco Aug 17, 2023

jcfranco Aug 17, 2023

anveshmekala Aug 18, 2023

anveshmekala Aug 18, 2023

geospatialem Aug 25, 2023

jcfranco commented Aug 17, 2023

anveshmekala commented Aug 17, 2023

geospatialem left a comment

geospatialem Aug 25, 2023

fix(rating): adds focus outline on click #7341

fix(rating): adds focus outline on click #7341

Conversation

anveshmekala commented Jul 19, 2023 • edited Loading

Summary

github-actions bot commented Jul 28, 2023

driskull left a comment

Choose a reason for hiding this comment

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Aug 17, 2023

Choose a reason for hiding this comment

jcfranco Aug 17, 2023

Choose a reason for hiding this comment

jcfranco Aug 17, 2023

Choose a reason for hiding this comment

anveshmekala Aug 18, 2023

Choose a reason for hiding this comment

anveshmekala Aug 18, 2023

Choose a reason for hiding this comment

geospatialem Aug 25, 2023

Choose a reason for hiding this comment

jcfranco commented Aug 17, 2023

anveshmekala commented Aug 17, 2023

geospatialem left a comment

Choose a reason for hiding this comment

geospatialem Aug 25, 2023

Choose a reason for hiding this comment

anveshmekala commented Jul 19, 2023 •

edited

Loading