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

List level Admin UI Hooks/Custom Actions #2033

Closed
gautamsi opened this issue Dec 2, 2019 · 6 comments
Closed

List level Admin UI Hooks/Custom Actions #2033

gautamsi opened this issue Dec 2, 2019 · 6 comments

Comments

@gautamsi
Copy link
Member

gautamsi commented Dec 2, 2019

In continuation to #1927 we do need hook/custom action in Admin UI which is specific to each list, see the example implementation in #1739.

this is not properly captured in #1452 (yet).

@MadeByMike
Copy link
Contributor

@gautamsi we're not going to have 'list-level' hooks. It will be the same API, we just need to add a call to an appropriate hook at the right point in the adminUI.

Once this happens we pass the hook a whole bunch of contextual information including the path and the items selected etc.... from there the hook can decide if it needs to render anything or bail. Make sense?

@gautamsi
Copy link
Member Author

gautamsi commented Dec 3, 2019

I see what you meant and the example from #1452

    pageLoad: ({path}) => { 
       /* runs on every adminUI page */ 
       if(location === `/admin/users/`) {
         /* runs on every users list page */ 
       } 
    }

one caveat is that this location things is not helpful as much. If there was a way to add list key (like User) instead of the location/path would be little more helpful.

Having this with list config would be much more cleaner. We can use the pattern decided in list level plugin which can extend the list config before initialization. or you are afraid that term hooks clashes with the list level config which already has hooks for different purpose. I still insist, call these actions or uiConfig or may be customizations rather than hooks, make our life easier and not difficult.

keystone.createList('User', {
fields: .....,
plugins: [atTracking()],
hooks: // this is where it gets ugly due to name hooks
});

We should

  • Keystone level configuration for admin-ui overall customization, like branding, custom pages, navbar, header, footer etc.
    • can also have custom actions which are applicable for all Lists.
  • List level configuration for selective custom actions.

@MadeByMike
Copy link
Contributor

But we can pass the list key. We can pass whatever values we want to a hook function.

@gautamsi
Copy link
Member Author

gautamsi commented Dec 3, 2019

For more convenient way, lets do the list level config in combination to keystone level config.
Hooks name is going to confuse many. Lets rename to one of suggestion above.

@MadeByMike
Copy link
Contributor

In general we try to avoid adding multiple ways to do the same thing. What we're aiming for is the right way with a consistent API that people can learn.

If the convention is that admin hooks are added to the keystone object and they receive a common set of parameters, as well as occasional hook-specific parameters. This allows people to learn the API better.

We want to avoid alternative methods that receive the same information in different ways.

We often talk about "API surface area" keeping this small and consistent leads to a more stable, more maintainable API that is easier to learn for users as well.

@gautamsi
Copy link
Member Author

gautamsi commented Dec 4, 2019

will have to wait and see how those page level hooks come alive with keystone level configuration and work in tandem.

hooks name is a 👎 from me.

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