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

TypeError in dropdown-group when disconnected early #10028

Closed
2 of 6 tasks
nwhittaker opened this issue Aug 9, 2024 · 3 comments
Closed
2 of 6 tasks

TypeError in dropdown-group when disconnected early #10028

nwhittaker opened this issue Aug 9, 2024 · 3 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps 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 - 3 A day or two of work, likely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - medium Issue is non core or affecting less that 60% of people using the library

Comments

@nwhittaker
Copy link
Contributor

Check existing issues

Actual Behavior

If a <calcite-dropdown-group> is removed from the DOM before it's done loading, it can throw a TypeError: Cannot read properties of null (reading 'querySelectorAll').

Expected Behavior

It does not throw an error.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/zYVdgqQ

Reproduction Steps

  1. Visit the repro and open the devtools console
  2. Click the Toggle dropdown group button and observe the TypeError log appears in the console

Reproduction Version

2.11.1

Relevant Info

Stack trace:

TypeError: Cannot read properties of null (reading 'querySelectorAll')
    at S.getGroupPosition (dropdown-group.tsx:166:29)
    at S.componentWillLoad (dropdown-group.tsx:85:31)
    at _e (index.js:1853:23)
    at ze (index.js:1649:22)
    at Array.r (index.js:1626:26)
    at M (index.js:185:13)
    at P (index.js:227:5)

I suspect treating this.el.parentElement here as optional would be sufficient:

this.el.parentElement.querySelectorAll("calcite-dropdown-group"),

Regression?

No response

Priority impact

impact - p2 - want for an upcoming milestone

Impact

The impact is largely with adding complexity to writing automated tests. In cases where the component is rendered as part of an acceptance test, its presence may not be important. However, the test still needs to wait for it to hydrate before performing an action that has the side-effect of removing the component.

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 Field Apps

@nwhittaker nwhittaker added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Aug 9, 2024
@github-actions github-actions bot added ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. calcite-components Issues specific to the @esri/calcite-components package. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone labels Aug 9, 2024
@geospatialem geospatialem added p - medium Issue is non core or affecting less that 60% of people using the library estimate - 3 A day or two of work, likely requires updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Aug 13, 2024
@driskull
Copy link
Member

I suspect treating this.el.parentElement here as optional would be sufficient:

Yeah I think that is likely the issue:

private getGroupPosition(): number {
    return Array.prototype.indexOf.call(
      this.el.parentElement.querySelectorAll("calcite-dropdown-group"),
      this.el,
    );
  }

Its looking for a parent element when its already been disconnected.

This should probably be refactored to not look up the DOM to get the group position. The group position wouldn't update if the group was moved position either.

@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Sep 6, 2024
@jcfranco jcfranco self-assigned this Feb 20, 2025
@jcfranco jcfranco added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Feb 20, 2025
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Feb 21, 2025
jcfranco added a commit that referenced this issue Feb 21, 2025
**Related Issue:** #10028

## Summary

This fixes an error caused by `dropdown-group`s looking up their parent
element. It updates `dropdown` to set the position directly on groups,
avoiding parent lookups.
@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 Feb 21, 2025
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco Feb 21, 2025
Copy link
Contributor

Installed and assigned for verification.

@DitwanP
Copy link
Contributor

DitwanP commented Feb 25, 2025

🍠 Verified on 3.1.0-next.17

@DitwanP DitwanP closed this as completed Feb 25, 2025
@DitwanP DitwanP 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 Feb 25, 2025
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 Field Apps Issues logged by ArcGIS Field Apps 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 - 3 A day or two of work, likely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone 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

5 participants