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

[BUG] popovers hide() function is called twice on dropdown-item click #2180

Closed
1 of 9 tasks
RasmusKjeldgaard opened this issue Apr 22, 2022 · 0 comments · Fixed by #2195
Closed
1 of 9 tasks

[BUG] popovers hide() function is called twice on dropdown-item click #2180

RasmusKjeldgaard opened this issue Apr 22, 2022 · 0 comments · Fixed by #2195
Assignees
Labels

Comments

@RasmusKjeldgaard
Copy link
Collaborator

Describe the bug

For a dropdown configured with [usePopover]="true", popovers hide() is called twice whenever an item in the dropdown is clicked.

This has caused issues for consumers, that see errors like below for subsequent calls to hide(), when dropdown is already hidden:

uncaught error in micro frontend DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
    at EmulatedEncapsulationDomRenderer2.removeChild (http://localhost:8783/default-node_modules_bankdata_relationsbank-sdk_fesm2015_bankdata-relationsbank-sdk-micro-fro-4d56c1.js:51308:20)
    at PopoverComponent.hide (http://localhost:8783/default-node_modules_bankdata_relationsbank-sdk_fesm2015_bankdata-relationsbank-sdk-micro-fro-4d56c1.js:84125:23)
    at PopoverComponent._backdropClick (http://localhost:8783/default-node_modules_bankdata_relationsbank-sdk_fesm2015_bankdata-relationsbank-sdk-micro-fro-4d56c1.js:84072:14)
    at PopoverComponent_click_HostBindingHandler (http://localhost:8783/default-node_modules_bankdata_relationsbank-sdk_fesm2015_bankdata-relationsbank-sdk-micro-fro-4d56c1.js:84181:141)

Describe how to reproduce the bug

  1. Check out repo, and edit a dropdown in the cookbook example to have [usePopover]="true".
  2. debug with breakpoint on popovers hide() function or console.log something in the function.
  3. See it being called twice, when an item within the dropdown is clicked.

Which Kirby version was used?

5.2.2

What was the expected behavior?

Only call hide() once, as the DOM is manipulated in the function, and elements might not be present on subsequent calls when the dropdown has been hidden.

Please complete the following information:

  • OS: [e.g. Windows, MacOS, iOS]
  • Browser [e.g. Chrome, Safari]
  • Browser version [e.g. 22]

Are there any additional context?

The reason that the function is called twice, is that a backdrop click is also recorded, due to the hostlistener in popover:
51f8d12#diff-0f54fa478d78f29a82f9c3830d735d4c5de57f30cf649945ea9d7c51fc0d2db8R48-R52

Unfortunately, the HostListener is added as part of a very large commit, so not much history exist on why it was added. It does not seem to fire when clicking the backdrop(!) and generally the popover functionality seems to work without it, at least when used in a dropdown.

Would be interesting to investigate if the listener can be removed entirely, if it is leftover functionality accidentally left in the component.


Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Verification

To make sure the bug is not intended behaviour; it should be verified by a member of team Kirby before moving on to implementation.

  • Make sure the NOT verified label has been removed by a member of team Kirby; do not proceed until this is done

Implementation

The contributor who wants to implement this issue should:

Review

Once the issue has been implemented and is ready for review:

@RasmusKjeldgaard RasmusKjeldgaard added bug NOT Prioritized Issue not yet prioritized and added to a Milestone NOT Verified Applied to bug reports that have not been verified by a member of @kirbydesign/kirby-guild and removed NOT Prioritized Issue not yet prioritized and added to a Milestone NOT Verified Applied to bug reports that have not been verified by a member of @kirbydesign/kirby-guild labels Apr 22, 2022
@alxzak alxzak added the 👶🏻 New For new issues before prioritisation and refinement label Apr 26, 2022
@RasmusKjeldgaard RasmusKjeldgaard moved this to 🔎 Review pending in Kirby Apr 27, 2022
@RasmusKjeldgaard RasmusKjeldgaard removed the 👶🏻 New For new issues before prioritisation and refinement label Apr 27, 2022
@RasmusKjeldgaard RasmusKjeldgaard self-assigned this Apr 27, 2022
@MadsBuchmann MadsBuchmann moved this from 🔎 Review pending to 👀 Review in progess in Kirby Apr 27, 2022
Repository owner moved this from 👀 Review in progess to ✅ Done in Kirby Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants