Skip to content

[Modal / Popover] Search Widget not receiving click event if Modal / Popover open after initial load #6581

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

Closed
macandcheese opened this issue Mar 9, 2023 · 5 comments
Assignees
Labels
4 - verified Issues that have been tested, confirmed as mitigated, and are ready to close. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (design) Issues logged by Calcite designers. estimate - 5 A few days of work, definitely requires updates to tests. has workaround Issues have a workaround available in the meantime. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - medium Issue is non core or affecting less that 60% of people using the library research Issues that require more in-depth research or multiple team members to resolve or make decision.

Comments

@macandcheese
Copy link
Contributor

macandcheese commented Mar 9, 2023

Actual Behavior

This was brought to our attention by a developer customer at Dev Summit. When using the Search Widget within a Modal that is not open on load, the Search Widget does not respond to the first click in its search results when within the Modal.

This is not evident if the Modal is open at load, and I was unable to reproduce with other elements besides the Search Widget. I also tested and was able to reproduce in Popover, leading me to believe there is some issue with the focus trap functionality.

Expected Behavior

As a developer customer, I can use the Maps SDK for Javascript Search Widget (https://developers.arcgis.com/javascript/latest/sample-code/widgets-search-customsource/) within a Modal, not open at load, and have the initial click on the results list within the Search Widget function as expected.

Reproduction Sample

Codepen

Reproduction Steps

1 - Navigate to Codepen
2 - Open the Modal via button
3 - Enter a search term
4 - Click on an item in the result list.
5 - The console log should not fire.
6 - Subsequent clicks should fire the console log.
7 - Edit the sample code to have the Modal open on load
8 - Observe that the console log DOES fire on same click
9 - Change the Modal element to a Popover and observe the same behavior.

Reproduction Version

1.0.7

Relevant Info

We went back and forth with Maps SDK on this one, it's replicable with the Search Widget, but not with other elements (Calcite or native) within the Modal / Popover.

Regression?

Priority impact

p1 - need for current milestone

Impact

Affecting multiple components, end users.

Esri team

Calcite (design)

@macandcheese macandcheese 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 Mar 9, 2023
@github-actions github-actions bot added the impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone label Mar 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

It looks like this issue is missing some information:

  • A codepen, codesandbox, or jsbin that reproduces the issue. Alternatively, a documentation sample can be used if the issue is reproducible. If you are experiencing build or Node related errors, please provide a GitHub repo for the sample.

Issues without reproduction samples may not be prioritized. If you were unable to create a sample, please try to answer any questions that arise once development begins. Thanks for your understanding.

@github-actions github-actions bot added incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. Calcite (design) Issues logged by Calcite designers. and removed incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. labels Mar 9, 2023
@geospatialem geospatialem added the estimate - 5 A few days of work, definitely requires updates to tests. label Mar 21, 2023
@geospatialem geospatialem added research Issues that require more in-depth research or multiple team members to resolve or make decision. p - medium Issue is non core or affecting less that 60% of people using the library needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Mar 30, 2023
@geospatialem geospatialem added this to the 2023 May Priorities milestone Mar 30, 2023
@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Mar 30, 2023
@geospatialem
Copy link
Member

Research will be conducted to determine next steps in May, as it relates to focus trap.

@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Mar 30, 2023
@jcfranco
Copy link
Member

Sharing workaround suggested by @driskull: using focus-trap-disabled mitigates this issue, but also allows users to tab into surrounding elements.

@jcfranco jcfranco added the has workaround Issues have a workaround available in the meantime. label Mar 30, 2023
driskull added a commit that referenced this issue Apr 11, 2023
- Uses the component's host element for the focus trap
- Sets up a mutation observer to update focusable elements when new elements are slotted
- If no focusable elements are found it will just call setFocus on the component.
driskull added a commit that referenced this issue Apr 12, 2023
**Related Issue:** #6581

## Summary

- Uses the component's host element for the focus trap. This seems to
fix the issue in the related PR. I guess we should always use the host
element in order to not run into any issues. Previously, it was using an
element inside the shadowRoot.
- Sets up a mutation observer to update focusable elements when new
elements are slotted
- Remove setting tabIndex on an element as a fallback
@driskull driskull added 3 - installed Issues that have been merged to the "dev" branch and/or are ready for QA/QC. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Apr 12, 2023
@github-actions github-actions bot assigned geospatialem and unassigned driskull Apr 12, 2023
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been tested, confirmed as mitigated, and are ready to close. and removed 3 - installed Issues that have been merged to the "dev" branch and/or are ready for QA/QC. labels Apr 13, 2023
@geospatialem
Copy link
Member

Verified with 1.3.0-next.3.

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 tested, confirmed as mitigated, and are ready to close. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (design) Issues logged by Calcite designers. estimate - 5 A few days of work, definitely requires updates to tests. has workaround Issues have a workaround available in the meantime. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - medium Issue is non core or affecting less that 60% of people using the library research Issues that require more in-depth research or multiple team members to resolve or make decision.
Projects
None yet
Development

No branches or pull requests

4 participants