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

feat(common): add group name to bind and unbindAll methods #1150

Merged
merged 1 commit into from Oct 25, 2023

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Oct 25, 2023

  • in order to bind and unbind a set of event listeners more easily, we can add an optional group name, for example this will be helpful to unbind all event related to sub-menus that gets closed while still keeping other event listeners in place when the parent menu remains open while sub-menus were closed

- in order to bind and unbind a set of event listeners more easily, we can add an optional group name, for example this will be helpful to unbind all event related to sub-menus while still keeping other event listeners in place
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #1150 (9c70633) into master (723e11c) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1150   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          245       245           
  Lines        17003     17010    +7     
  Branches      6114      6117    +3     
=========================================
+ Hits         17003     17010    +7     
Files Coverage Δ
...ckages/common/src/services/bindingEvent.service.ts 100.00% <100.00%> (ø)

@ghiscoding ghiscoding merged commit 6c3b90e into master Oct 25, 2023
5 checks passed
@ghiscoding ghiscoding deleted the feat/event-bind-group branch October 25, 2023 19:08
ghiscoding added a commit that referenced this pull request Oct 26, 2023
- in order to bind and unbind a set of event listeners more easily, we can add an optional group name, for example this will be helpful to unbind all event related to sub-menus while still keeping other event listeners in place
ghiscoding pushed a commit that referenced this pull request Oct 26, 2023
- previous PR #1150 brought this new feature but it was found that if the bounded events to unbind are at the end of the array and we called `unbindAll`, it wasn't removing all espected events because calling `splice` whithin the logic was reindexing the array causing an index offset and it missed unbounding them all.
- the best approach is to loop in reverse order which has no consequences on the array indexes and will not cause offset
ghiscoding added a commit that referenced this pull request Oct 26, 2023
…nes (#1152)

- previous PR #1150 brought this new feature but it was found that if the bounded events to unbind are at the end of the array and we called `unbindAll`, it wasn't removing all espected events because calling `splice` whithin the logic was reindexing the array causing an index offset and it missed unbounding them all.
- the best approach is to loop in reverse order which has no consequences on the array indexes and will not cause offset

Co-authored-by: ghiscoding <Ghislain.Beaulac@se.com>
ghiscoding pushed a commit that referenced this pull request Oct 26, 2023
- previous PR #1150 brought this new feature but it was found that if the bounded events to unbind are at the end of the array and we called `unbindAll`, it wasn't removing all espected events because calling `splice` whithin the logic was reindexing the array causing an index offset and it missed unbounding them all.
- the best approach is to loop in reverse order which has no consequences on the array indexes and will not cause offset
ghiscoding pushed a commit that referenced this pull request Oct 27, 2023
- another small issue found after implementing previous PR #1150, `groupName` was only added to single event `bind`, however it should also be supported by `.bind` with multiple events provided as an array
ghiscoding added a commit that referenced this pull request Oct 27, 2023
#1157)

- another small issue found after implementing previous PR #1150, `groupName` was only added to single event `bind`, however it should also be supported by `.bind` with multiple events provided as an array

Co-authored-by: ghiscoding <Ghislain.Beaulac@se.com>
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.

None yet

2 participants