Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
269 stars 75 forks source link

[modal] TypeError when calling `setFocus` with dist-custom-elements #6188

Closed mpayson closed 1 year ago

mpayson commented 1 year ago

Actual Behavior

When calling setFocus on calcite-modal with dist-custom-elements, the TypeError below gets thrown. The error stems from focusTrapEl being undefined when focusFirstTabbable is called here, so maybe the fix from #5749 does not work with the custom-elements output type? Also, it does look like setFocus gets called internally here and focuses the close button as expected, so consuming apps may not need to call it?

focusTrapComponent.js:76 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'tagName')
    at getCandidatesIteratively (focusTrapComponent.js:76:1)
    at tabbable (focusTrapComponent.js:418:1)
    at focusFirstTabbable (focusTrapComponent.js:1250:1)
    at Modal.open.setFocus (calcite-modal.js:226:1)

Expected Behavior

setFocus method waits until the component has loaded and the TypeError is not thrown

Reproduction Sample

https://github.com/mpayson/calcite-components-examples/tree/set-focus-issue

Reproduction Steps

  1. Clone the repository and checkout the set-focus-issue branch
  2. cd react && npm install && npm start
  3. Open developer console
  4. Click button to open the modal, see console error
  5. Switch the loading strategy here by commenting lines 5-6 and un-commenting lines 7-8
  6. Refresh the page
  7. Click button to open the modal, don't see console error

Reproduction Version

beta.99

Relevant Info

No response

Regression?

No response

Impact

No response

Esri team

ArcGIS Online

benelan commented 1 year ago

Note: this isn't specific to React, it repro's in the vite sample as well:

index.html ```html Calcite Vite Click me!
Demo
hi!
```
main.js ```js import { setAssetPath } from "@esri/calcite-components/dist/components"; import "@esri/calcite-components/dist/components/calcite-button"; import "@esri/calcite-components/dist/components/calcite-modal"; import "@esri/calcite-components/dist/calcite/calcite.css"; import "./style.css"; setAssetPath(location.href); ```
jcfranco commented 1 year ago

@mpayson Thanks for the excellent issue description. Installed a workaround to avoid the error, while we look into creating a repro case. Fortunately, as you pointed out, focus works as expected after the open transition.

github-actions[bot] commented 1 year ago

Installed and assigned for verification.

geospatialem commented 1 year ago

Verified with master.