-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add catalogAddMenu filter to CatalogCategory #3722
Conversation
ac128f6
to
bcd777d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
74643fe
to
494cbbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>
Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>
494cbbf
to
a461ac5
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
public addMenuFilter(fn: AddMenuFilter): Disposer { | ||
this.filters.add(fn); | ||
|
||
return once(() => void this.filters.delete(fn)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only nit would be that if the category is removed, these should be removed too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Do you mean if I call the Disposer
returned by CatalogCategoryRegistry::add
, I should remove the filters from the CatalogCategory
instance? Doesn't sound right to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah probably not.
catalogAddMenu
context menu items. Extension can e.g. filter certain menu items from the context menu.The filter can be removed with the returned disposer:
CatalogCategory
classes.