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

.setFocus() not working on several components in newer Chrome versions #10394

Closed
2 of 6 tasks
maxpatiiuk opened this issue Sep 26, 2024 · 7 comments
Closed
2 of 6 tasks
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 5 A few days of work, definitely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - medium Issue is non core or affecting less that 60% of people using the library

Comments

@maxpatiiuk
Copy link
Member

Check existing issues

Actual Behavior

Calling .setFocus() on calcite-action-group has no effect.

Expected Behavior

If .setFocus() method on the calcite-action-group element is called, I am expecting the focus to move to first focusable element.

Reproduction Sample

https://codepen.io/maxpatiiuk/pen/wvVBQXw?editors=1010

Reproduction Steps

  1. Open the codepen
  2. Open console
  3. See the the active element before and after the .setFocus() call remains the same

Reproduction Version

2.13

Relevant Info

The codepen works like expected in Chrome 119. Does not work in Chrome 128.

Calcite has E2E test for setFocus in calcite-action-group. Since Stencil's Puppeteer is using Chrome 119, the test is passing. Upon updating Chrome to 129, the test is failing:

focusable(actionGroupHTML, { shadowFocusTargetSelector: "calcite-action" });


Another example:

  1. On the docs page for action-bar, from the list of samples select "Action Bar with groups"
  2. Open DevTools and find the calcite-action-group element rendered by the sample
  3. Call .setFocus() method on that element in the devtools console.
  4. In Chrome 119 that would focus the first slotted in action. In Chrome 129 this has no effect.

Possible solution:

Setting delegatesFocus: true for calcite-action element seems to fix this issue.
You already have it on calcite-action-group, but might need to add it to calcite-action too:

Regression?

No response

Priority impact

impact - p3 - not time sensitive

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Maps SDK for JavaScript

@maxpatiiuk maxpatiiuk added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 26, 2024
@github-actions github-actions bot added ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. calcite-components Issues specific to the @esri/calcite-components package. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive labels Sep 26, 2024
@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Oct 5, 2024

Also affects calcite-date-picker.setFocus()

Fixed by adding delegatesFocus:true to calcite-date-picker-month-header

@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Oct 5, 2024

input-date-picker is affected too:

In Chrome 119 clicking on the arrow-right icon between the two text inputs will move focus to the first input.

Screenshot 2024-10-05 at 13 29 47

In Chrome 128 that does not happen.

That causes an E2E test failure because this click no longer moves focus to the first input:


My workaround thus was to update the test per new behavior:

-     await input.click(); 
+     const toggleIcon = await input.find(`>>> .${CSS.toggleIcon}`);
+     await toggleIcon.click();

However, a different solution is required if you want to preserve the Chrome 119 behavior.

@maxpatiiuk maxpatiiuk changed the title [calcite-action-group] .setFocus() not working in newer Chrome versions .setFocus() not working on several components in newer Chrome versions Oct 5, 2024
@maxpatiiuk maxpatiiuk added p2 - want for current milestone User set priority status of p2 - want for current milestone impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive and removed impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p2 - want for current milestone User set priority status of p2 - want for current milestone labels Oct 5, 2024
@maxpatiiuk
Copy link
Member Author

Also affecting calcite-inline-editable - that component by default tries to do this.el.focus(), which does not appear to move focus to the slotted-in input anymore.

Calling this.inputElement?.setFocus(); instead works, but only if I add delegatesFocus:true to calcite-input (and same would have to be done for all the other inputs).

In general, it appears that delegatesFocus:true should be added to most (all?) Calcite components.

@maxpatiiuk
Copy link
Member Author

Also affects <calcite-menu> test:

describe("focusable", () => {
focusable(html`<calcite-menu><calcite-menu-item text="calcite"></calcite-menu-item></calcite-menu>`, {
focusTargetSelector: "calcite-menu-item",
});
});

Fixed by adding delegatesFocus to calcite-menu-item and changing this.el.focus() in calcite-menu to this.menuItems[0]?.focus();

@jcfranco
Copy link
Member

jcfranco commented Oct 9, 2024

In general, it appears that delegatesFocus:true should be added to most (all?) Calcite components.

We should revisit our findings, but based on our earlier exploration on delegatesFocus, I wouldn't suggest enabling this by default for all components.

@jcfranco jcfranco self-assigned this Oct 10, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. estimate - 5 A few days of work, definitely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Oct 10, 2024
@jcfranco jcfranco added this to the 2024-10-29 - Oct Milestone milestone Oct 10, 2024
jcfranco pushed a commit that referenced this issue Nov 1, 2024
Make minor adjustments to Calcite code to have post-migration tests
pass.

Changes that need to be done post-migration are encoded in the following
codemod files in the arcgis-web-components repository:
- /packages/support-packages/stencil-to-lit/src/code/fileSpecific.ts
-
/packages/support-packages/stencil-to-lit/src/component/fileSpecific.ts

At this point, I am finishing up the failing tests in the S components
and will be pretty much done with all A-S component (with exception of
#10394 and
#10495)
jcfranco added a commit that referenced this issue Nov 4, 2024
…10676)

**Related Issue:** #10394

## Summary

This uses `focusFirstTabbable` to ensure focus works properly.
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Nov 4, 2024
Copy link
Contributor

github-actions bot commented Nov 4, 2024

Installed and assigned for verification.

@github-actions github-actions bot assigned DitwanP and unassigned jcfranco Nov 4, 2024
@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Nov 6, 2024
@geospatialem
Copy link
Member

Verified in 3.0.0-next.7 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 5 A few days of work, definitely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - medium Issue is non core or affecting less that 60% of people using the library
Projects
None yet
Development

No branches or pull requests

4 participants