Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gravypower created an issue. See original summary.

Gravypower’s picture

Gravypower’s picture

Gravypower’s picture

Status: Active » Needs review
Gravypower’s picture

Issue summary: View changes
Gravypower’s picture

added check to see if the placeholder settings exist.

ifrik’s picture

This patch works for me.

mikeker’s picture

Status: Needs review » Fixed

@Gravypower, thank you for the feature request and patch! Unfortunately, there's a fair bit more that needs to be handled when adding new options to BEF -- in this case we needed default values for easy upgrades and a schema entry so the value is translatable.

I've committed a slightly different take on this which will give them option to specify placeholder text for text-based filters (filters that extend StringFilter). I'm not sure if I should include filters that extend NumericFilter as well. Please feel free to open a followup for that if you feel they should be included.

  • mikeker committed 80233c4 on 8.x-3.x
    Issue #2846909 by mikeker, Gravypower: Placeholder support
    

Status: Fixed » Closed (fixed)

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

ekes’s picture

So the schema change was important.

What was the reason to restrict the filter type to Drupal\views\Plugin\views\filter\StringFilter? I guess it only makes sense in some cases? However, I've got a filter of type Drupal\search_api\Plugin\views\filter\SearchApiFulltext which had the placeholder, and it makes sense there. I don't think it's going to be possible to list all types that it does make sense with - or is there some other way to know about the field?

mikeker’s picture

@ekes, you're correct, the restriction to StringFilter types (and, it could be argued, NumericFilter as well...) is to prevent the option from showing up for filters that do not support placeholders (eg: boolean).

SearchAPI is somewhat problematic as they extend FilterPluginBase instead of StringFilter. There are other places in BEF where we special case SearchAPI's filters and it looks like this will be one more... Please open a new issue for this. Thanks.

mikeker’s picture

Forgot to credit my sponsoring company...

mikeker’s picture