sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Show filter #6217

Closed kirya-dev closed 4 years ago

kirya-dev commented 4 years ago

In version sonata-admin 3.72 i have a problem. Im configure default filter value, but in this case not visible on page. But checkbox in dropdown it checked that means filter is applied. Its hiding by admin.isDefaultFilter(), but default value not null and filter must visible.

Logic in template: https://github.com/sonata-project/SonataAdminBundle/blob/bc07812be8f1eff56edbcd9baaefbb0de7ebfa46/src/Resources/views/CRUD/base_list.html.twig#L278-L280

Pass display on filterActive.. but sonata-filter on filterVisible. Maybe keep only filterVisible?

It suggest to do

<div class="form-group {% block sonata_list_filter_group_class %}{% endblock %}" id="filter-{{ admin.uniqid }}-{{ filter.name }}" sonata-filter="{{ filter.isVisible() ? 'true' : 'false' }}" style="display: {% if  filter.isVisible() %}block{% else %}none{% endif %}">
namespace Sonata\DoctrineORMAdminBundle\Filter;

class Filter
{
/...
    public function isVisible()
    {
        return $this->active || $this->options['show_filter'] === true;
    }

Also refactor this: https://github.com/sonata-project/SonataAdminBundle/blob/bc07812be8f1eff56edbcd9baaefbb0de7ebfa46/src/Resources/views/CRUD/base_list.html.twig#L246-L258

Must be

 <ul class="dropdown-menu" role="menu"> 
     {% for filter in admin.datagrid.filters|filter(filter => filter.options['show_filter'] is same as(true) or filter.options['show_filter'] is null) %} 
         <li> 
             <a href="#" class="sonata-toggle-filter sonata-ba-action" filter-target="filter-{{ admin.uniqid }}-{{ filter.name }}" filter-container="filter-container-{{ admin.uniqid() }}"> 
                 <i class="fa {{ filter.isVisible() ? 'fa-check-square-o' : 'fa-square-o' }}"></i> 
                 {% if filter.label is not same as(false) %} 
                     {{ filter.label|trans({}, filter.translationDomain ?: admin.translationDomain) }} 
                 {% endif %} 
             </a> 
         </li> 
     {% endfor %} 
 </ul> 
VincentLanglet commented 4 years ago

I think this is similar to https://github.com/sonata-project/SonataAdminBundle/issues/6137 And also https://github.com/sonata-project/SonataAdminBundle/pull/5745

When default filter were introduced, they were meant to be hidden for @core23. https://github.com/sonata-project/SonataAdminBundle/pull/5745#issuecomment-555598687

I personally think this behavior is unnatural, and we should always show used filters. What do you think about it @kirya-dev ?

There is also currently a bug about the checkbox. But before trying to fix it, maybe we should decide what is the more UX-friendly behavior for default filters.

cc @sonata-project/contributors

kirya-dev commented 4 years ago

Im apologize 1) Filter visible on form (also checked in dropdown) when is applied. 2) Force filter visible of form by show_filter option. (Rename option?) 3) All filters always visible in dropdown (if need hide there no call $datagridMapper->add(...)

Rename method isVisible to isVisibleOnForm? if default value is not null and it hide - that wrong UI. I think that default value is null because it hide

kirya-dev commented 4 years ago

@VincentLanglet what do you think?

VincentLanglet commented 4 years ago

@VincentLanglet what do you think?

Sorry, I didn't understand your previous message. Can you explain what you meant ?

I was confirming that if you use configureDefaultFilter, the filter was apply, hidden, but the checkbox was checked (and this is a bug).

But I was thinking that an hidden filter apply by default is not really UX-friendly. To me, every filters applied by default should be visible, so maybe we should consider changing (fixing ?) this UX-behavior. Then if we really want to add a hidden filter feature, methods should be called hiddenFilter, not defaultFilter. I was expecting some opinion from @sonata-project/contributors on this.

kirya-dev commented 4 years ago

To me, every filters applied by default should be visible, so maybe we should consider changing (fixing ?) this UX-behavior.

Yes. Im agree with you.

If filter is Applied (no matter is Default) filter is should be Visible!

kirya-dev commented 4 years ago

There have a really bug: When filter is applied -> They checked in dropdown of available filters. But they really dont showing on form.. Also im see empty block with Filter button. In other cases this block fully hiding.

VincentLanglet commented 4 years ago

There have a really bug: When filter is applied -> They checked in dropdown of available filters. But they really dont showing anything.. Also im see empty block with Filter button.

Yes, there is two way to fix this:

I personally prefer the second option. Do you want to start a PR on this @kirya-dev ?

The Changelog will be

Changed:
- Stop to hide default filters in order to provide a more natural UX-behavior.

Deprecate
- Abstract:: isDefaultFilter method.
kirya-dev commented 4 years ago

@VincentLanglet PR is ready!