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

fix(client): on click menus now open the menu again #4650

Merged
merged 2 commits into from
Jan 31, 2023

Conversation

naftis
Copy link
Collaborator

@naftis naftis commented Jan 30, 2023

Closes #4644

Adding the useCapture -flag fixes the issue:

useCapture:
A boolean value indicating whether events of this type will be dispatched to the registered listener before being dispatched to any EventTarget beneath it in the DOM tree. Events that are bubbling upward through the tree will not trigger a listener designated to use capture. Event bubbling and capturing are two ways of propagating events that occur in an element that is nested within another element, when both elements have registered a handle for that event. The event propagation mode determines the order in which elements receive the event. See DOM Level 3 Events and JavaScript Event order for a detailed explanation. If not specified, useCapture defaults to false.

@naftis
Copy link
Collaborator Author

naftis commented Jan 30, 2023

@rikukissa @Zangetsu101 to be honest I'm not 100% sure why the React update needed this change to make the menus function. Another way they worked when I changed 'click' to 'mousedown'. Maybe it's something related to https://blog.saeloun.com/2021/07/08/react-17-event-delagation.html React 17 attaching events to root DOM container instead of document node... 🤔

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Base: 89.72% // Head: 89.74% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (f9c2e4f) compared to base (b9505c2).
Patch coverage: 92.30% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4650      +/-   ##
===========================================
+ Coverage    89.72%   89.74%   +0.01%     
===========================================
  Files          722      722              
  Lines       127873   127897      +24     
  Branches     11564    11609      +45     
===========================================
+ Hits        114739   114778      +39     
+ Misses       13128    13113      -15     
  Partials         6        6              
Impacted Files Coverage Δ
packages/client/src/components/ProtectedPage.tsx 77.44% <0.00%> (-0.89%) ⬇️
...ent/src/views/CorrectionForm/CorrectionSummary.tsx 83.02% <0.00%> (ø)
...views/OfficeHome/requiresUpdate/RequiresUpdate.tsx 92.69% <ø> (-0.05%) ⬇️
...nt/src/views/RegisterForm/review/ReviewSection.tsx 75.19% <91.11%> (+0.43%) ⬆️
packages/client/src/components/ErrorBoundary.tsx 100.00% <100.00%> (ø)
packages/client/src/components/Notification.tsx 84.49% <100.00%> (+0.24%) ⬆️
packages/client/src/components/Page.tsx 96.23% <100.00%> (-0.05%) ⬇️
packages/client/src/components/ScrollToTop.tsx 100.00% <100.00%> (ø)
...ckages/client/src/components/TransitionWrapper.tsx 100.00% <100.00%> (ø)
...s/form/DocumentUploadfield/DocumentListPreview.tsx 98.20% <100.00%> (ø)
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Zangetsu101
Copy link
Collaborator

facebook/react#24657 (comment)
facebook/react#20074
It seems that from 18 and up, if you add a event listener in a click/type handler, that handler will also be triggered for the click/type that added the handler in the first place

@Zangetsu101
Copy link
Collaborator

And as the handler was added in the target phase of the event(which means the capture phase has already passed), the new handler doesn't get called in this case and we get our desired result.

@Zangetsu101 Zangetsu101 merged commit 7384479 into develop Jan 31, 2023
@Zangetsu101 Zangetsu101 deleted the fix-menu-problem branch January 31, 2023 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking avatar from the right top corner doesn't open profile menu
2 participants