-
Notifications
You must be signed in to change notification settings - Fork 326
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
Adding static methods for creating standard ToolbarActions #6707
base: 2.6
Are you sure you want to change the base?
Conversation
60803a7
to
4bef5f0
Compare
4bef5f0
to
847097d
Compare
@mamazu Nice idea, like it that the toolbar action are also represented in PHP 👍. I personally would even go the way of have an own class per ToolbarAction, like we already have for the For |
I think the idea of using a I think I would implement it in the same way as we did for views to keep things consistent. For the moment, I think I would just implement two factories - |
i would go with the same as we have for the views. own classes and factories. E.g.:
and
The resolving of the classes itself from the factory should then be done later when the view-factory will be resolved. |
The |
I mean creating seperate classes for all the actions definitely sounds like a good idea. But for the factory structure I am not really sure if this is the best way. One thing that I like to keep in mind (and also one of the reasons I have created this pull request) was to simplify the view creation process for "simple" pages that only have a list view and an edit form. So maybe something like class ToolbarFactory {
public function createForForm(array $buttonNames): array {
$buttons = [];
foreach($buttonNames as $buttonName) {
$button = new ToolbarAction($buttonName);
if ($toolbarAction->isSuitableForForm()) {
$buttons[] = $button;
}
}
return $buttons;
}
public function createForList(array $buttonNames): array {
$buttons = [];
foreach($buttonNames as $buttonName) {
$button = new ToolbarAction($buttonName);
if ($toolbarAction->isSuitableForList()) {
$buttons[] = $button;
}
}
return $buttons;
}
} and usage would be like: $this->toolbarFactory->generateForList([ToolbarAction::ADD, ToolbarAction::DELETE, ToolbarAction::EXPORT]);
$this->toolbarFactory->generateForForm([ToolbarAction::SAVE, ToolbarAction::DELETE]); And for more complex cases the user can still add their own buttons to the list. |
I think I have an idea that might work. So taking the idea from alexander to make the buttons into classes has two advantages:
An example: interface FormToolbarAction {}
interface ListToolbarAction {}
class SaveFormToolbarAction implements FormToolbarAction
{
public function __construct(array $options) {
}
// ... code goes here
}
class ExportListToolbarAction implements ListToolbarAction
{
public function __construct(array $options) {
}
// code goes here
} Then you can have a ToolbarBuilderFactory (similar to the ViewBuilderFactory) that creates either a ListToolbarActionBuilder or a FormToolbarActionBuilder and usage would be like so. $listToolbarActions = $this->toolbarBuilder->createListToolbar()
->add(new ExportListToolbarAction())
// ->add(...)
->toArray()
;
$listToolbarActions = $this->toolbarBuilder->createFormToolbar()
->add(new SaveFormToolbarAction())
// ->add(...)
->toArray()
; If the buttons then also encode what permissions they need to be displayed then this should also drastically reduce the complexity of the generation of the toolbar. Because in the current system it is easy to
|
What's in this PR?
In this pull requests I created static methods on the
ToolbarAction
class to easily createToolbarAction
objects.Why?
There are a few reasons why. It's shorter and more concise to read. But the main reason is that you don't have to rely on magic strings anymore. So the user doesn't have to know that the delete button is the
sulu_admin.delete
button string.And since the old feature isn't touched at all, it does not break any backwards compatibility.
Example Usage
To Do