From working on #1851086: Replace admin/people with a View, it would be good if you could add an option for exposed forms to 'hide reset button'. This would hide the reset button when no exposed input has been added. I.e. It only appears when there are things to reset.

This could do with some work for sure (and tests), but as a quick POC, something like this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

I don't need the !empty($this->options['reset_button_hide']) in the second part of the conditional.

damiankloip’s picture

Issue tags: -Needs tests
FileSize
4.33 KB

And a couple of assertions.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -34,6 +34,7 @@ protected function defineOptions() {
+    $options['reset_button_hide'] = array('default' => FALSE, 'bool' => TRUE);

I'm wondering whether the user expects a JS behavior for hiding the button?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -58,6 +59,13 @@ public function buildOptionsForm(&$form, &$form_state) {
+      '#description' => t('Hide the reset button when no exposed filter values have been selected'),

Maybe a dot at the end of the description?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -58,6 +59,13 @@ public function buildOptionsForm(&$form, &$form_state) {
+      '#default_value' => $this->options['reset_button_hide'],
+    );

This needs #states, the same as the rest button label.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -236,10 +237,23 @@ function exposed_form_alter(&$form, &$form_state) {
-      if (isset($form['reset'])) {
-        $form['reset']['#weight'] = 10;

What an odd logic...

damiankloip’s picture

FileSize
1.06 KB
4.49 KB

Thanks for the review.

I'm wondering whether the user expects a JS behavior for hiding the button?

Do you mean you think we should implement this with js?

Rerolled to include your other suggestions.

What an odd logic...

Yep, views has lots of stuff like that from the ages :)

dawehner’s picture

Well, not sure whether this flickering would cause issues, so maybe this could be an option? It feels like we should ask bojhan,
whether such an UI part should be done in javascript as well.

damiankloip’s picture

I will try and ask Bojhan today.

tstoeckler’s picture

Just an innocent question, feel free to ignore: Is there any reason we should make this an option instead of just hardcoding it. It seems showing a Reset button without any filters applied is a very strange edge-case, unless I'm missing something.

tim.plunkett’s picture

I also wondered why this wasn't just the hardcoded behavior. When would you want that shown?

Dave Reid’s picture

Agreed with #7 and #8, I would think this would be enabled by default, or hardcoded.

Crell’s picture

If your filter form is ajax-enabled, having the Reset button appear and disappear could be jarring, and possibly do weird things to your layout if your CSS isn't expecting it.

That said, I agree this is a "do what Bojhan and yoroy tell us to do" moment.

damiankloip’s picture

I like the option. I spoke to Bojhan about this briefly yesterday, I will get him to comment in here.

yoroy’s picture

Only showing a reset button when there's something to reset, that's just plain good manners no? It should at least be the default.

I'm sure that if we all try really hard we can come up with a use case for wanting to show a reset when there's nothing to reset ("look! if you'd filter something, you will be able to reset it!"). But where would this setting live and is it really worth the extra checkbox in an already complex UI?

*Edit:* tstoeckler: good thinking, excellent question! :-)

Bojhan’s picture

I agree with this approach, by default we should hide it. If there is a usecase where you want to expose it by default, then that should be possible programatically but lets not add another checkbox.

damiankloip’s picture

FileSize
3.73 KB
3.88 KB

ok, Bojhan also agrees that we should just make this default behaviour, so let's remove the options and just set the access to FALSE in the form so it's easy to alter etc..

dawehner’s picture

Well, it still feels better to

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -236,10 +229,27 @@ function exposed_form_alter(&$form, &$form_state) {
+    // Get an array of exposed filters, keyed by identifier option.
+    foreach ($this->view->filter as $id => $handler) {
+      if ($handler->canExpose() && $handler->isExposed() && !empty($handler->options['expose']['identifier'])) {
+        $exposed_filters[$handler->options['expose']['identifier']] = $id;
       }
     }
+    $all_exposed = array_merge($exposed_sorts, $exposed_filters);
...
+    if (!array_intersect_key($all_exposed, $this->view->exposed_input)) {
+      $form['reset']['#access'] = FALSE;
+    }

Shouldn't we move this logic into the if (!empty() bit, as it's not needed unless you have a reset button?

damiankloip’s picture

FileSize
942 bytes
3.86 KB

You are totally right, that didn't make any sense. Thanks!

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.phpundefined
@@ -236,8 +229,26 @@ function exposed_form_alter(&$form, &$form_state) {
+    // Get an array of exposed filters, keyed by identifier option.
+    foreach ($this->view->filter as $id => $handler) {
+      if ($handler->canExpose() && $handler->isExposed() && !empty($handler->options['expose']['identifier'])) {
+        $exposed_filters[$handler->options['expose']['identifier']] = $id;
+      }
+    }
+    $all_exposed = array_merge($exposed_sorts, $exposed_filters);

Also that could move into the if.

damiankloip’s picture

Let's move it all in there! good plan.

sun’s picture

Status: Needs review » Reviewed & tested by the community
damiankloip’s picture

Title: Add an exposed form option to hide reset button when no exposed input has been added » Hide reset button when no exposed input has been added
webchick’s picture

Status: Reviewed & tested by the community » Needs work

Hm. This doesn't appear to be working 100% for me.

Steps:

- Edited the front page view, exposed both filters.
- Checked the "Include reset button" setting under Advanced > Exposed form style > Settings (took me 10 minutes to find that :P)
- Save.
- Go to front page.
- Despite no filters selected / applied, Reset button still appears.

I do confirm that if I click apply, then Reset, the reset button goes away. But each time I go back to the front page with no filters applied, reset button is there until I click it.

dawehner’s picture

FileSize
4.85 KB

- Checked the "Include reset button" setting under Advanced > Exposed form style > Settings (took me 10 minutes to find that :P)

Is that caused by the problem, that it's in the advanced area?

I tried to do exactly the steps you described, and mhhh the reset button did not appeared :(
Here is the frontpage config i tested with.

damiankloip’s picture

@webchick, I can't replicate this either I'm afraid :( It seems no one else can either. Is this still doing the same for you?

damiankloip’s picture

Status: Needs work » Needs review

Setting back to needs review; we can't replicate webchick's issue :( So more people to test would be great.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

I can not reproduce #21 either. The reset button works as expected.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.phpundefined
@@ -61,14 +61,28 @@ public function testResetButton() {
+    // Test the 'reset_button_hide' option.
+    $view = views_get_view('test_reset_button');
+    $view->setDisplay();
+    $exposed_form = $view->display_handler->getOption('exposed_form');
+    $exposed_form['options']['reset_button_hide'] = TRUE;
+    $view->display_handler->setOption('exposed_form', $exposed_form);
+    $view->save();

The reset_button_hide option no longer exists.

alexpott’s picture

BTW I couldn't reproduce #21 either.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.75 KB
3.79 KB

Good point alex, I remove that from the test and modified the order so that it made more sense now we don't have the option.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Loos great.

alexpott’s picture

Title: Hide reset button when no exposed input has been added » Change notice: Hide reset button when no exposed input has been added
Status: Reviewed & tested by the community » Fixed

Committed 1f39cc4 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.