fix(panel): tweak focusable content area #10141

jcfranco · 2024-08-22T00:16:35Z

Related Issue: #10022

Summary

Updates panel to have a single focusable container when it has scrolling content.

Noteworthy changes

Improves test coverage for both scrolling and non-scrolling content.
Updates disabled test helper to clear focus targets between tab and click tests

alisonailea

🎉

macandcheese · 2024-08-27T15:46:21Z

The conditionally-applied tab index looks great! And should apply to Tab most likely as well :)

I think we might need to explore some other options for showing the visual focus outline here - might require some fake containers / pseudo elements - this focusable area is hard to discern and has a lot of overlap with the content slotted inside:

…() interactions

driskull

👍

jcfranco · 2024-09-04T07:15:09Z

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

-    expect(calcitePanelClose).toHaveReceivedEventTimes(1);
-  });
-
-  it("should not close when Escape key is prevented and closable is true", async () => {


@driskull Due to the contents of scrollingContentHtml, this test is only covered under the scrolling use case.

We could add coverage for this test for both scrolling and non-scrolling content by adding focusable elements to the content. Does that approach make sense? If so, would we want to keep scrolling content and non-scrolling content as the main test groups or do we need an additional group to cover scrolling with and without interactive content?

jcfranco · 2024-09-04T08:09:28Z

For posterity, after some extensive troubleshooting, it turns out that not having a set width/height was affecting disabled tests (namely this assertion from the prevents focusing via keyboard and mouse test) in the CI despite tests passing locally in both headless and headful modes for both macOS and Linux.

**Related Issue:** #10022 ## Summary Updates `panel` to have a single focusable container when it has scrolling content. ### Noteworthy changes * Improves test coverage for both scrolling and non-scrolling content. * Updates `disabled` test helper to clear focus targets between tab and click tests

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 2.12.1</summary> ## [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) ### Bug Fixes * **loader:** Restore animation ([#10184](#10184)) ([52081ca](52081ca)) * **panel:** Tweak focusable content area ([#10141](#10141)) ([fa18b70](fa18b70)) </details> <details><summary>@esri/calcite-components-angular: 2.12.1</summary> ## [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </details> <details><summary>@esri/calcite-components-react: 2.12.1</summary> ## [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </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>

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **loader:** Restore animation ([#10184](#10184)) ([52081ca](52081ca)) * **panel:** Tweak focusable content area ([#10141](#10141)) ([fa18b70](fa18b70)) </details> <details><summary>@esri/calcite-components-angular: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **@esri/calcite-components-angular:** Synchronize components versions * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </details> <details><summary>@esri/calcite-components-react: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **@esri/calcite-components-react:** Synchronize components versions * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </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>

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **loader:** Restore animation ([#10184](#10184)) ([52081ca](52081ca)) * **panel:** Tweak focusable content area ([#10141](#10141)) ([fa18b70](fa18b70)) </details> <details><summary>@esri/calcite-components-angular: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **@esri/calcite-components-angular:** Synchronize components versions * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </details> <details><summary>@esri/calcite-components-react: 2.12.1</summary> [2.12.1](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2024-09-04) * **@esri/calcite-components-react:** Synchronize components versions * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0 to 2.12.1 </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>

… align with browsers (#10242) **Related Issue:** #10238 ## Summary Fixes a regression caused by #10141 where clicks on scrollers would show the focus outline. ### Notes * this behavior aligns with Chrome/Firefox's latest default focus on scrolling content behavior * `:focus-visible` [isn’t supported in Safari 15.0 - 15.3](https://caniuse.com/?search=focus-visible), but given Safari 18 is expected this year, this is a minor visual inconsistency for unsupported versions. * cleans up focus styles that should have been updated by #10141. * no tests were added as we currently don't cover interactive focus-related styling

fix(panel): tweak focusable content area

2ec1b7a

github-actions bot assigned jcfranco Aug 22, 2024

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

jcfranco added 2 commits August 27, 2024 01:13

improve scrolling/non-scrolling test coverage

32f873b

merge dev

12bee02

jcfranco requested review from driskull and macandcheese August 27, 2024 08:15

jcfranco marked this pull request as ready for review August 27, 2024 08:17

alisonailea approved these changes Aug 27, 2024

View reviewed changes

macandcheese mentioned this pull request Aug 30, 2024

[Panel] Focus outline obscures slotted content #10154

Open

6 tasks

jcfranco added 9 commits August 30, 2024 14:45

Merge branch 'dev' into jcfranco/10022-tidy-up-scroller-content-tabbing

a64cff2

tidy up test

8726142

remove invalid test

90ccd29

update disabled helper to check different targets for click and click…

d669873

…() interactions

use same default body click focus target

c2c810d

fix tests

cf84dd9

troubleshoot panel & flow-item tests

a863056

restore previous assertions

c90a34e

add context to assertion error messages

9d1a73a

driskull approved these changes Sep 3, 2024

View reviewed changes

jcfranco added 5 commits September 3, 2024 10:30

test rest of assertions

da1c313

restore assertion and add delay before asserting failing assertion

370e29e

log extra info

ca43a99

tweak log info

241634f

fix prop access

b546802

geospatialem added this to the 2.12.1 patch milestone Sep 3, 2024

jcfranco added 2 commits September 3, 2024 17:07

tweak log info

52ba6c7

bump puppeteer

ecabbe0

jcfranco requested a review from benelan as a code owner September 4, 2024 03:26

jcfranco added 10 commits September 3, 2024 20:33

merge dev

ff2cc38

bump puppeteer properly

e145c64

add large delay before assertion

e4964e1

tweak page size and sandbox

51e3d27

roll back sandbox options to pinpoint fix

d7d77e8

roll back dimensions from test config

1cc1f79

remove troubleshooting log messages and tidy up context messages

4238007

roll back puppeteer version

ddc4d16

revert lockfile

4bcc814

remove focused-test pattern for troubleshooting

f246b82

jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label Sep 4, 2024

jcfranco commented Sep 4, 2024

View reviewed changes

fix test where clicking on body would restore focus on component

c9346af

jcfranco merged commit 3dd4b28 into dev Sep 4, 2024
10 checks passed

jcfranco deleted the jcfranco/10022-tidy-up-scroller-content-tabbing branch September 4, 2024 08:04

benelan mentioned this pull request Sep 4, 2024

chore: release main #10212

Merged

benelan mentioned this pull request Sep 4, 2024

chore: release main (#10212) #10213

Merged

jcfranco mentioned this pull request Sep 6, 2024

fix(panel, flow-item): hide focus-outline when scrolling via mouse to align with browsers #10242

Merged

calcite-admin mentioned this pull request Sep 30, 2024

chore: release main #10457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(panel): tweak focusable content area #10141

fix(panel): tweak focusable content area #10141

jcfranco commented Aug 22, 2024 •

edited

Loading

alisonailea left a comment

macandcheese commented Aug 27, 2024

driskull left a comment

jcfranco Sep 4, 2024

jcfranco commented Sep 4, 2024

fix(panel): tweak focusable content area #10141

fix(panel): tweak focusable content area #10141

Conversation

jcfranco commented Aug 22, 2024 • edited Loading

Summary

Noteworthy changes

alisonailea left a comment

Choose a reason for hiding this comment

macandcheese commented Aug 27, 2024

driskull left a comment

Choose a reason for hiding this comment

jcfranco Sep 4, 2024

Choose a reason for hiding this comment

jcfranco commented Sep 4, 2024

jcfranco commented Aug 22, 2024 •

edited

Loading