-
Notifications
You must be signed in to change notification settings - Fork 33
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
base: main
Are you sure you want to change the base?
[V5]: FE APIs #56
Conversation
Can you give us examples of how the injection zone replacement would be UI wise? |
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? |
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.
Please do not remove injection zones, but allow a graceful upgrade to the new api.
|
||
type HeaderAction = ButtonAction | SelectAction; | ||
|
||
interface ButtonAction { |
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.
we need more controls.
- control of the color of the button with.
- permission support
- rbac conditional support
- visibility controls
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.
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.
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.
@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 { |
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.
same as the Button action
) => void; | ||
icon?: React.ElementType; | ||
disabled?: boolean; | ||
dialog?: DialogOptions | NotifiationOptions | ModalOptions; |
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.
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
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.
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"
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.
@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 |
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.
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. |
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.
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.
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. |
Thanks for this RFC,
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 🙌 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! |
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 also disagree when you say that "not every Strapi user modifies their product to such a high degree.". 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 🫶 |
Introduce typed imperative APIs to inject
DocumentActions
,SideBarPanels
andDocumentHeaderActions
as opposed toInjectionZones
in V5.You can read it here