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

replace pluginBkmrksEdit hook with various different hooks #353

Open
Aradiv opened this issue Mar 5, 2020 · 2 comments
Open

replace pluginBkmrksEdit hook with various different hooks #353

Aradiv opened this issue Mar 5, 2020 · 2 comments

Comments

@Aradiv
Copy link

Aradiv commented Mar 5, 2020

This hook gets triggered all the time with various different targets making every implementation that check this hook having a huge switch/case or if to cover the actions.

Available actions

  • reset
  • import
  • open
  • close
  • remove
  • add

also a lot of the actions have different targets

  • all
  • map
  • portal
  • folder

and some of the actions are only valid with one of the targets making the target pretty much obsolete.

reset/import => always all
open/close => always folder
add/remove => portal/map/folder

My Suggestion would be ro replace this 1 hook with 6 seprate hooks so hook functions just need to run for their respective case and not for everything (eg most things that run on that hook probably don't care about open/close folders) also handling reset/import is a completly different thing from add/remove an entry in the bookmarks

@johnd0e
Copy link
Contributor

johnd0e commented Mar 5, 2020

My Suggestion would be ro replace this 1 hook with 6 seprate hooks

Shouldn't we keep old hook too, for compatibility?

@Aradiv
Copy link
Author

Aradiv commented Mar 5, 2020

yes but we should mark it deprecated an warn people when they are using it.

generally we need a deprecation ruling on how and when we want to break BC and remove deprecated stuff to get IITC evolved to a more modern system.

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

No branches or pull requests

2 participants