Closed codeclipse closed 2 years ago
You don't need to override this method because you can call setListMode
in the configure()
method.
It is not possible, because it requires that request
is available, which is not when configure()
method is called.
Additionally, I believe that this would break storing list mode in user's session. If mode would be set manually in configure()
it would be used always, unless _list_mode
would be send in request.
It is not possible, because it requires that request is available, which is not when configure() method is called.
I already got some issues about this. When I want to configure an admin according to the request, I have to call the initialize
method a second time. Don't know if we can do something about this.
Additionally, I believe that this would break storing list mode in user's session. If mode would be set manually in configure() it would be used always, unless _list_mode would be send in request.
Oh I see, it's the 'list'
part you want to change.
I think we should deprecate the show_mosaic_button
and instead introduce a list_modes
config.
It could default to the value
[
'list' => ['class' => 'fas fa-list fa-fw', 'default' => true],
'mosaic' => ['class' => 'fas fa-th-large fa-fw'],
]
Then, if you want to change you can configure
[
'list' => ['class' => 'fas fa-list fa-fw'],
'mosaic' => ['class' => 'fas fa-th-large fa-fw', 'default' => true],
]
or without the mosaic
[
'list' => ['class' => 'fas fa-list fa-fw', 'default' => true],
]
or even create your own modes.
The setListModes
call would be made with the config value instead or starting with TaggedAdminInterface::DEFAULT_LIST_MODES
and unsetting the mosaic if show_mosaic_button
is false.
And in the admin, we would just default to the key of the default mode.
WDYT @codeclipse ; do you want to work on this ?
Yes, I was thinking about that 'list'
.
Your idea sounds pretty good, I'll try to work on that later.
Feel free to ping me with a draft PR, I'll help you about how to introduce the deprecation in a BC way.
@codeclipse I opened https://github.com/sonata-project/SonataAdminBundle/pull/7725 if you want to try it.
You'll have to call setListModes
with
[
'mosaic' => [
'icon' => '<i class="fas fa-th-large fa-fw" aria-hidden="true"></i>',
],
'list' => [
'icon' => '<i class="fas fa-list fa-fw" aria-hidden="true"></i>',
],
]
instead of
[
'list' => [
'icon' => '<i class="fas fa-list fa-fw" aria-hidden="true"></i>',
],
'mosaic' => [
'icon' => '<i class="fas fa-th-large fa-fw" aria-hidden="true"></i>',
],
]
and since now it take the first key as the default value, it should work.
@VincentLanglet looks good.
Thanks for taking look at this. Unfortunately I don't have much time lately, but I will check it as soon as I can.
Feature Request
Since
AbstractAdmin::getListMode()
is final, there is no way to configure which list mode should be used by default. It is now hardcoded within this method.https://github.com/sonata-project/SonataAdminBundle/blob/013102f2241454046315d59be760a426ac974fe3/src/Admin/AbstractAdmin.php#L1771-L1778
I believe that adding getter/setter in AbstractTaggedAdmin with additional tag attribute (f.ex. default_list_mode) would be a great solution here.