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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Oct 25, 2023
Merged

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

merged 1 commit into from
Oct 25, 2023

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Oct 25, 2023 •
edited
Loading

  • 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

@ghiscoding-SE
feat(common): add group name to bind and unbindAll methods
9c70633 
- 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
@ghiscoding @ghiscoding-SE
feat(common): add group name to bind and unbindAll methods (#1150)
8a397f8 
- 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
@ghiscoding-SE
fix(common): unbindAll with a group name should remove all tagged ones
5d81965 
- 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 ghiscoding mentioned this pull request Oct 26, 2023
Merged
ghiscoding added a commit that referenced this pull request Oct 26, 2023
@ghiscoding @ghiscoding-SE
fix(common): unbindAll with a group name should remove all tagged o…
5014354 
…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
@ghiscoding-SE
fix(common): unbindAll with a group name should remove all tagged ones
1403ecf 
- 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
@ghiscoding-SE
fix(common): calling bind with multiple events should add group name
d977a91 
- 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 ghiscoding mentioned this pull request Oct 27, 2023
Merged
ghiscoding added a commit that referenced this pull request Oct 27, 2023
@ghiscoding @ghiscoding-SE
fix(common): calling bind with multiple events should add group name (
9023b54 
#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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ghiscoding @ghiscoding-SE