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

DBC22-1219: use popup ref when icon clicked #194

Merged
merged 2 commits into from
Nov 27, 2023
Merged

Conversation

bcgov-brwang
Copy link
Collaborator

DBC22-1219: use popup ref when icon clicked to prevent popup onClick event got triggered.

DBC22-1219: use popup ref when icon clicked to prevent popup onClick event got triggered.
Copy link
Collaborator

@ray-oxd ray-oxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ref here essentially functions like a lock, but I can see some issues with the way it's implemented. Namely:

  • If someone opens the popup on the top-half of the screen, they'd have to click twice, once to get rid of the lock, second time for the actual handler
  • The lock is not being reset on popup close. This may not have functional impact yet but is potentially bug-inducing

We are on the right track though:

  • Remove: the else clause/block in mapPopup.js starting line 55
  • Remove: the if clause in Map.js starting line 417, we want to set the cameraPopupRef here no matter what
  • Add: reset the lock on successful navigation
  • Add: reset the lock on popup close
  • Add: reset the lock on a 0.5s-1s timer after popup creation, right after we set cameraPopupRef at around line 418 in Map.js

@bcgov-brwang
Copy link
Collaborator Author

For this change, "Remove: the else clause/block in mapPopup.js starting line 55", it will cause the first hit of the pop up does not work. We can deleted the return statement, but the ref still needs to be set to null to make it work.

DBC22-1219: Rework done. Updated according to the comments.
@ray-oxd ray-oxd merged commit bd50a92 into main Nov 27, 2023
@ray-oxd ray-oxd deleted the bugfix/DBC22-1219-2 branch November 27, 2023 21:51
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.

2 participants