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

[V5]: FE APIs #56

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[V5]: FE APIs #56

wants to merge 1 commit into from

Conversation

joshuaellis
Copy link

Introduce typed imperative APIs to inject DocumentActions, SideBarPanels and DocumentHeaderActions as opposed to InjectionZones in V5.

You can read it here

@joshuaellis joshuaellis added the v5 label Sep 19, 2023
@joshuaellis joshuaellis self-assigned this Sep 19, 2023
@Boegie19
Copy link

Can you give us examples of how the injection zone replacement would be UI wise?

@Boegie19
Copy link

I like the idea but why remove the InjectionZones and not build this feture ontop of InjectionZones. so it can be an InjectionZones or documentActions, SideBarPanels and DocumentHeaderActions

# Unresolved questions

- Do we want to allow DocumentActions to be written as if they’re react components but return a data structure? This allows actions to be even more powerful, but their implementation in Strapi becomes potentially more complex.
- Should we live with injection zones in V4 and only deprecate them or completely remove them and do the additional work to define new APIs for the affected areas?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not remove injection zones, but allow a graceful upgrade to the new api.


type HeaderAction = ButtonAction | SelectAction;

interface ButtonAction {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need more controls.

  • control of the color of the button with.
  • permission support
  • rbac conditional support
  • visibility controls

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you? I think if you want all those things you should be using the addDocumentHeaderAction(actionFn: HeaderActionFn): void; so you could do something like:

addDocumentHeaderAction(() => {
  const { canRead } = useRBAC()

  if(!canRead){
    return null
  }

  return {}
})

I think the disabled property is probably helpful for sure, the color of the button, im not sure – that's a product decision it could be viable.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshuaellis Lets say my use cease would be syncing document with something external then I would like to have a green button if fully synced orange when syncing and red when it is not synced it would allow you to communicate with the end user without them having to press the button first.

RBAC makes sence still the pemissions field like for side bar should exist

ctx: EditViewContext
) => DocumentAction[];

interface DocumentAction {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as the Button action

) => void;
icon?: React.ElementType;
disabled?: boolean;
dialog?: DialogOptions | NotifiationOptions | ModalOptions;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotifiationOptions makes no send how do I know what notification to send if it happens on interaction with the button I don't even know if what I tried failed or succeeded NotifiationOptions should be a function that returns a notification

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's definitely use-cases e.g. your document action is to sync with an external data source, you just want a notificaiton that says "synced"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshuaellis it would always be synced or error. if it says synced while it has an error that would not be a good user experience.

or can dialog also be a function?


Overall, we want to move towards more developer friendly APIs that are quicker to use, more intuitive yet do not cause concerns for the usability of our product.

# Detailed proposal

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am missing a information table in here so we can add stuff to this. https://gyazo.com/d40ae81220023684f1bc479fbd630b62

) => TConfig[];
```

Because the function receives the previous list of configs, we expect the complete list of configs to be returned, this means if a plugin required it could remove a configuration, or mutate a current configuration building upon one another e.g. the publish action would be able to be mutated by the `i18n` plugin correctly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see 1 issue with this implementation if there are no controls for the order the configs are read in how can I chance something that i18n changed if it is loaded after my function is done.

@Boegie19
Copy link

We also need an easy way for plugins to add the special zones you made to the plugin like you can do with injection zones.

@t-fritsch
Copy link

t-fritsch commented Nov 29, 2023

Thanks for this RFC,
V4 was a huge step back regarding frontend customization, compared to the freedom offered in V3.
If I understand it well, this RFC propose a new way to add components in some pre-defined places (especially the header and edit view).
This is great but V5 should also provide solutions to not only "add" stuff, but also affect existing parts of the UI :

  • in the list view
  • in the menu
  • in the dashboard
  • in popups
  • etc.

There are numerous issues/posts about this :

What is so frustrating with v4 is that the backend part can be customized quite fully : with routes, controllers, middlewares, services, we can do (almost) ANYTHING. This is perfect 🙌
And when you switch to the frontend part you only get a tiiiiiiny space to play with (and always adding stuff, not editing or removing). It's like switching from paradise to jail 😰😅

Please don't forget that if the developers that work with strapi are good enough to produce good backend code, we are also capable of doing some solid React Frontend development.
I understand that it won't be possible in the very first V5 releases to match FE customization possibilities with the level of customization given BE-side, but please try to give us some freedom back 🙏

@joshuaellis
Copy link
Author

Please don't forget that if the developers that work with strapi are good enough to produce good backend code, we are also capable of doing some solid React Frontend development.

No one's forgetting that, no one is saying that developers can't write frontend code. The issue primarily is it creates an unstable product where even the smallest changes we make can break other people's admin panels – you see this consistently with issues on the BE already. So what we're doing is weighing up customisation with stability because not every Strapi user modifies their product to such a high degree.

Hopefully, should these APIs be well received we can move to exposing more in the future because what we're doing is giving the ability to customise without compromising usability which is products core concern that I sympathise with.

Thank you for all your feedback though, it's well appreciated & we'll keep it all in mind!

@t-fritsch
Copy link

Thanks for your quick reply @joshuaellis ! I realize you maybe misunderstood my intentions : I wasn't trying to accuse anyone of anything. Sorry if I wasn't clear. 🙏

I'm just emphasizing that there is a HUGE gap between the liberty you give BE side and FE side, and this cannot be solved by locking FE developers on so small areas like injections zones (If I understood the RFC the new system is quite the same).
I think we can all imagine the frustration it would raise if you did the same thing BE side ie. allow developers to add stuff only in small places of the API or BE code, but not to override/remove core things. This is exactly the same on frontend.

I also disagree when you say that "not every Strapi user modifies their product to such a high degree.".
I think that modification of the dashboard for example is needed by almost every user, the fact that the majority of nowadays strapi users accomodate to that doesn't mean they don't need it. Same can be said about the menus, the default rendering of relations in list view, or the way dates are formatted, etc.

I understand that this is important to not break things on every update and I've not digged deeper enough in strapi core to know what the solution would be, I'll leave it to the strapi team, but I hope you'll find a way to give back to developers better control of the UI, and not only on empty places but on existing part of the UI 🫶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants