Allow min & max exposed numeric/date filters to be configured

Any updates on this request? We are trying to override the Min and Max labels now on a government site.

@michael.friend you can use merge request diff as patch

https://git.drupalcode.org/project/drupal/-/merge_requests/2391/diffs.patch

Hi, i'll try to work on those errors :)

Now the tests are passing without errors :)

I'll review it.

Applied the #14 patch and works as well.

Steps:

- Added a filter to Post date to a view.
- Configured the operator to is between, and check the: Expose this filter to visitors.
- Set the min and max labels.

Everything is working, I suggest moving to RTBC.

Applied patch #14 on 9.5.x. The Min and Max label configuration fields are appearing in view configuration after applying this patch.The label change is reflecting in the view page as well.

Attaching screenshot below.

After patch:

I will review that!

I applied the pacth #14 and everything worked fine.

We should add some test coverage of the new capability. And we should have a change notice to announce it to the community.

Plus what about existing views when they are resaved? Do they change max_label and min_label added? I.e. do we need to think about upgrade path?

Also meant to add to #22 that this change makes lots of sense and will be useful.

Patch #14 also applies to Drupal 10.1.x-dev

#22 mentions the need for tests and a change record. Not ready for review yet.

Added tests, please review

Moving back to NW for change record.

Also with the schema change it will need an update hook.

Ive just tested this on D11.2.4 and it all works.

This is exactly what i was looking for.

Thanks.
Works great for Drupal 10.5.2

Thanks everyone for the work on this. Updates made:

1. Updated fork to include 11.x branch
2. Applied the patch from #28 (which has already been confirmed as tested)
3. Tested it myself and confirmed its working correctly
4. Wrote a change record https://www.drupal.org/node/3560313

Updated tag now that change record exists

Updated issue summary with steps to reproduce (was missing)

The code changes look ok to me. I tested the changes with number, date, and timestamp fields. In all cases the new label fields were added to the views filter form.

I also tested this issue's compatibility with #3204558: Timestamp field type uses numeric filter and argument plugins rather than date in Views, which will change the filter handler for timestamp fields. Since the date handler extends the numeric filter, there's no problem. It retains the new label elements after applying the additional changes.

The issue summary needed to be updated with the changes to the user interface. I reused the image from the change record. The change record itself looks good.

In response to the questions of #22:

Plus what about existing views when they are resaved? Do they change max_label and min_label added? I.e. do we need to think about upgrade path?

The defaults of "Min" and "Max" are used. There was no issue with my test view after the patch was applied, but not after resaving the view. The defaults were displayed in the exposed form and the filter plugin forms. Everything continued to work after resaving. From what I can see we don't need an upgrade path because the defaults were set up correctly.

I am marking this as RTBC, but there is something that I don't like about this. The use of the abbreviations "Min" and "Max" doesn't sit right with me. I'm fairly certain that my accessibility reviewer would flag it as a problem. But it's used throughout the elements of the Numeric filter plugin. I didn't dig far into it, but it's been this way in the admin interface a long time since before Views was added to Core. I wonder if they ever underwent usability review. The terms are vague, as are the purposes of the fields they define. It's no wonder that people want to relabel them on the front end. The only reason I'm not suggesting the new element labels be changed now is because the terms are ubiquitous. But I am suggesting to the committer that a follow-up be opened for evaluating whether they should be changed throughout the form.

Thanks for your work on this issue! Providing review both in my capacity as a committer and as a Views subsystem maintainer (and Views in Drupal Core initiative team member from back in 2012-2013).

1. We still need automated test coverage in the canonical MR. Was it maybe applied to a hidden branch by mistake?
    

2. I agree with the previous review that "Min label" and "Max label" are (ironically) not great labels. We generally don't use abbreviations like that in the user interface; it's against our content style guidelines. That said, in the screenshot in #18, we already have "Min placeholder" and "Max placeholder", so I guess Views is already violating the content style guidelines. (Not for the first time.)

   I can actually answer this:

   I didn't dig far into it, but it's been this way in the admin interface a long time since before Views was added to Core. I wonder if they ever underwent usability review.

   Several rounds of usability review, in fact, before both D7 and D8. But it was after the fact when Views already existed for years as a thing a developer built for developers. So, we had to prioritize. Views had so many features that we had to focus on the most-used elements (and on removing as many elements as possible).

   In this case, I would recommend we come up with better, clearer labels for the fields that follow our content style guidelines, and then have a followup to fix the existing fields above it to match.
    

3. Speaking of usability review. In the old screenshots from back in 2022, there is a description on the fields that is redundant with the labels without adding any real information. We did a lot of work during Views in Drupal Core to remove these redundant descriptions from the Views UI. I don't see these descriptions in the schema in the MR, though, so are they being inherited, or is it a mismatch between the screenshot and the MR?

   If they are being inherited, it's worth either overriding it or a followup to fix the parent to not have redundant noise everywhere in the Views UI. Because if that snuck in somehow, it undid weeks of VDC and usability team collaboration and that makes me sad.

   In either case, let's get updated screenshots to confirm the behavior, since it's been a few years.
    

4. Finally... There's the decision of whether to add these elements to Views in core, or whether the functionality should live in contrib. I'm on the fence about that still, but I think answers to the above questions and improved usability will help us decide. I'm leaning toward including it, because while this does add UI bloat for site builders, it also feels like the previous issue in #2625136: Fix label visibility and add wrapper container for exposed numeric/date filters with multiple form elements got us sort-of-halfway-there for the actual site users' experience.

Hmm. After thinking about this for a few minutes, I am also wondering if the default values for these labels are also not-the-best. Certainly, we need to change the defaults to use whole words. That will cut the need to override this by at least 10%.

For dates, I think we should just ship default config of "Start date" and "End date".

I need to check why the tests are failing, but out of time this morning.

Updated screenshots of date & non-date to show label defaults if not changing any config, did this by going to the admin > content view and adding exposed filters.

For Date (authored on / created):

For Node ID (nid):

And the output in the exposed filter:

On the descriptions: the old screenshots were a mismatch with the MR at the time. The descriptions ("Title for the Min field.", "Title for the Max field.") were added in the original patch code — they were not inherited from the parent FilterPluginBase. In the current MR they have been removed, in line with the usability work you're describing.

I have updated the issue summary and change record with an updated screenshot reflecting the current MR + described the labels as Minimum and Maximum and Start date and End date for date + have added test coverage for that.

Tests also passing now, there was a workspace test failure that was unrelated, and just hitting rerun was enough for that to pass.

I didn't change the placeholders from "Min placeholder" nor the field description there as its not really whats discussed in the issue summary, but it would be easy of course to extend the scope to that if you prefer.

Finally I removed the Needs tests and Needs screenshots tags since those are added, apologies if I shouldn't have done that myself (not sure the etiquette).

Can we get an update hook since these are new keys being added to views.

Are there any test views we will need to update?

Thanks for the feedback!

I added an upgrade path based on the existence of min_placeholder rather than filter plugin ID to capture extensions to numeric/date. I set it up to maintain the status quo of 'Min' and 'Max' to be least disruptive to existing sites - hope that makes sense. So essentially new sites would take advantage of Minimum/Maximum/Start date/End date and existing sites could follow the change request to take advantage of that.

I also updated all existing test views that contained min_placeholder to add in the min_label & max_label

Ah and I updated the change record again to indicate that e.g. contrib module maintainers that have test views in their config install should also do such an update

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Resolved merge conflict

Legitimate failure, looks like fixtures in the test coverage need an update again

Okay back to needs review, tests now pass again (fixtures updated)

Upgrade path looks good.

Needs follow up was added above with

I would recommend we come up with better, clearer labels for the fields that follow our content style guidelines, and then have a followup to fix the existing fields above it to match.

But I see the label has been updated so does the follow up need to be a in general update? Happy to open just not 100% sure what the scope should be.

Manually tested by adding a 'created' filter to the default content view on a standard install.
Verified that the form appears as the screenshot and changing the labels works as expected on the frontend.

Definitely a feature that comes up often kudos!

I merged back to 11.x and resolved the conflicts.

Committed c6e7444 and pushed to main. Thanks!
Committed f00162f and pushed to 11.x. Thanks!

I think this broke 11.x: https://git.drupalcode.org/project/drupal/-/jobs/12137259

Maybe we can just add #[IgnoreDeprecations] to the test?

Views Add Filter Min Max Labels Update (Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdate)
     ⚠ Views post update filter min max labels
    
    1 test triggered 7 deprecations:
    
    1) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    The module 'text_with_summary' is deprecated. See https://www.drupal.org/node/3223395#s-text-with-summary-field
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    2) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    Drupal\help\Plugin\Search\HelpSearch is deprecated in drupal:11.4.0 and is removed from drupal:12.0.0. Instead, use \Drupal\search_help\Plugin\Search\HelpSearch. See https://www.drupal.org/node/3617771
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    3) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    Drupal\node\Plugin\Search\NodeSearch is deprecated in drupal:11.4.0 and is removed from drupal:12.0.0. Instead, use \Drupal\search_node\Plugin\Search\SearchNode. See https://www.drupal.org/node/3590298
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    4) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    Drupal\user\Plugin\Search\UserSearch is deprecated in drupal:11.5.0 and is removed from drupal:12.0.0. Instead, use \Drupal\search_user\Plugin\Search\SearchUser See https://www.drupal.org/node/3590230
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    5) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    The module 'search_help' is deprecated. See https://www.drupal.org/node/3223395#s-search
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    6) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    The module 'search_node' is deprecated. See https://www.drupal.org/node/3223395#s-search
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    7) /builds/project/drupal/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
    The module 'search_user' is deprecated. See https://www.drupal.org/node/3223395#s-search
    
    Triggered by:
    
    * Drupal\Tests\views\Functional\Update\ViewsAddFilterMinMaxLabelsUpdateTest::testViewsPostUpdateFilterMinMaxLabels
      /builds/project/drupal/core/modules/views/tests/src/Functional/Update/ViewsAddFilterMinMaxLabelsUpdateTest.php:36
    
    OK, but there were issues!
    Tests: 1, Assertions: 180, Deprecations: 7.

I agree, I just came here to report the same problem.

Fix per #58: https://git.drupalcode.org/project/drupal/-/merge_requests/17093

BTW, if you, like me, were wondering why the MR status and comments aren't showing up in this issue thread: https://git.drupalcode.org/project/drupalorg/-/work_items/3607033

Yes, ignoring the deprecation warnings for the 11.x branch makes sense on this update test.

Committed 667ea93 and pushed to 11.x. Thanks!

Update and publish the change record.