NEW: Allow search field customisation #9341

unclecheese · 2019-11-26T22:39:52Z

By default, GridFieldFilterHeader chooses the first field in the searchable fields array for the search bar. This allows you to customise what that field is.

Notes

Assumes NEW Search across multiple searchable fields by default #10382 will be merged in first.
The reviewer should make a judgment call as to whether this is a useful enhancement following NEW Search across multiple searchable fields by default #10382. I (Guy) think it probably is, as it allows people to customise their SearchContext and GridFields further and separately.

Parent issue

ModelAdmin SearchForm filters only on the first field #9356

maxime-rainville

We need to make it easier and clearer for developers to know how to use and customise the new search interface. This is a good step in that direction.

I'm thinking we should provide a way for them to do it directly on their DataObjects, but this PR is a good intermediary step.

src/Forms/GridField/GridFieldFilterHeader.php

unclecheese · 2019-11-26T23:32:30Z

I'm not totally sold on adding it to the DataObject API. I think we've already got such a bloated DataObject "config", and we have a long history of just chucking concerns on to that pile that could be handled elsewhere.

maxime-rainville · 2019-11-27T01:03:23Z

That's probably a wider discussion for an RFC. This PR by itself doesn't preclude our duplicate these efforts. I'm fine with merging this PR once unit tests have been added and maybe some doc.

unclecheese · 2020-01-13T01:41:26Z

Good feedback, @maxime-rainville .. have added the primary_search_field config, plus tests and docs.

Cheddam · 2020-01-13T04:55:04Z

Likely doesn't impact this PR going through as-is, but #9356 talks about setting multiple fields to be searchable in the main field. Thinking we could change these new APIs in the future to optionally accept an array to achieve that issue's goals?

lekoala · 2020-01-13T13:20:10Z

@Cheddam thanks for tracking this. Multiple field is indeed necessary in my opinion, as it is very unconvenient for the user to go through the dropdown in order to search on other fields. I think the common expectation of all my customers is that "typing something in the search bar matches on all relevant fields", not only on a single field.

Cheddam · 2020-01-15T02:21:04Z

@lekoala I appreciate that this feature would be really useful to users, but it'll need to be tackled in a separate PR - I just wanted to make @unclecheese aware of it, and check that this API is going to suit expansion to support it in the future.

maxime-rainville

I'm happy with the PR and I've re-tested that it works. I'm thinking some of the doc could use simpler language, but otherwise I'm happy.

docs/en/02_Developer_Guides/00_Model/11_Scaffolding.md

docs/en/02_Developer_Guides/12_Search/01_Searchcontext.md

emteknetnz · 2020-07-02T04:26:56Z

@unclecheese when you get a chance would you be able to implement Maxime's recommended docs changes? We should be able to get this merged once those are in

lekoala · 2021-05-12T09:02:37Z

For reference
#9836

GuySartorelli · 2022-07-01T03:32:11Z

I have rebased this on 4, resolved merge conflicts, and updated the docs.
The documentation assumes #10382 will be merged before this. There will likely be a merge conflict between these two PRs' docs, as well.

The approval came before #10382 was created and may not apply with it as context

GuySartorelli · 2022-07-07T01:32:59Z

Note: The documentation portion should not be merged here. I have started the process of migrating docs to a new repository so the docs will need to be removed from this PR and a new PR raised in the new repo instead.

GuySartorelli · 2022-07-07T21:55:48Z

Docs changes have been moved to silverstripe/developer-docs#6

emteknetnz · 2022-07-11T22:13:18Z

src/Forms/GridField/GridFieldFilterHeader.php

+     * The name of the default search field
+     * @var string|null
+     */
+    protected $searchField = null;


Suggested change

protected $searchField = null;

protected ?string $searchField = null;

emteknetnz · 2022-07-11T22:18:30Z

src/Forms/GridField/GridFieldFilterHeader.php

+    /**
+     * @return string|null
+     */


Suggested change

/**

* @return string|null

*/

emteknetnz · 2022-07-11T22:18:41Z

src/Forms/GridField/GridFieldFilterHeader.php

+    /**
+     * @param string $field
+     * @return $this
+     */


Suggested change

/**

* @param string $field

* @return $this

*/

emteknetnz · 2022-07-11T22:18:47Z

src/Forms/GridField/GridFieldFilterHeader.php

+    public function setSearchField(string $field): self
+    {
+        $this->searchField = $field;
+


Suggested change

Subjective but okay. Done.

emteknetnz

~~Could you move the contents of this PR into #10382 and close this one, that way there's only a single framework PR which makes testing a lot easier~~ < keep them separate

GuySartorelli · 2022-07-11T22:29:08Z

If you agree the changes in this PR are worthwhile keeping then yes, I'll merge the two PRs. Though the combined PR should not be squashed in that case. These are two related but separate changes.

emteknetnz · 2022-07-11T22:43:30Z

Oh right, sorry got confused with all the PRs. Hold off for now, see how the other PR looks on it's own and then in a better position to judge if this is worthwhile. Doesn't seem like we should combine the two in either case

emteknetnz

I'm thinking close this one and add an allowlist style config into #10382 would be better

GuySartorelli · 2022-07-25T05:34:39Z

We came to the decision not to use an allow-list configuration in the end: #10382 (comment)

emteknetnz

Tested locally, works good

maxime-rainville reviewed Nov 26, 2019

View reviewed changes

src/Forms/GridField/GridFieldFilterHeader.php Outdated Show resolved Hide resolved

unclecheese force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch from b4cdd16 to d6fde8f Compare January 13, 2020 01:14

Cheddam mentioned this pull request Jan 13, 2020

ModelAdmin SearchForm filters only on the first field #9356

Closed

7 tasks

maxime-rainville self-requested a review January 22, 2020 04:59

maxime-rainville self-assigned this Jan 22, 2020

maxime-rainville force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch from a1eaed5 to 8f0ec8c Compare January 23, 2020 03:15

maxime-rainville previously approved these changes Jan 23, 2020

View reviewed changes

docs/en/02_Developer_Guides/00_Model/11_Scaffolding.md Outdated Show resolved Hide resolved

docs/en/02_Developer_Guides/00_Model/11_Scaffolding.md Outdated Show resolved Hide resolved

docs/en/02_Developer_Guides/12_Search/01_Searchcontext.md Outdated Show resolved Hide resolved

emteknetnz added affects/v4 change/minor complexity/medium impact/medium type/enhancement labels Sep 6, 2020

emteknetnz removed the change/minor label Jun 20, 2022

GuySartorelli force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch from 8f0ec8c to 18cf766 Compare July 1, 2022 03:30

GuySartorelli unassigned maxime-rainville Jul 1, 2022

GuySartorelli mentioned this pull request Jul 7, 2022

RFC: Move documentation out of framework into a dedicated repo #10215

Closed

7 tasks

GuySartorelli force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch from 18cf766 to c884549 Compare July 7, 2022 21:52

GuySartorelli mentioned this pull request Jul 7, 2022

DOC Document GridFieldFilterHeader general field customisation silverstripe/developer-docs#6

Merged

emteknetnz requested changes Jul 11, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch 2 times, most recently from d2dbe74 to 24ea191 Compare July 11, 2022 22:33

emteknetnz requested changes Jul 12, 2022

View reviewed changes

GuySartorelli mentioned this pull request Jul 12, 2022

NEW Search across multiple searchable fields by default #10382

Merged

NEW: Allow search field customisation in GridFieldFilterHeader

07a6c11

GuySartorelli force-pushed the pulls/4/come-on-baby-make-it-search-so-good branch from 24ea191 to 07a6c11 Compare July 25, 2022 05:33

emteknetnz approved these changes Aug 1, 2022

View reviewed changes

emteknetnz merged commit c466ca5 into silverstripe:4 Aug 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NEW: Allow search field customisation #9341

NEW: Allow search field customisation #9341

unclecheese commented Nov 26, 2019 •

edited by GuySartorelli

maxime-rainville left a comment

unclecheese commented Nov 26, 2019

maxime-rainville commented Nov 27, 2019 •

edited

unclecheese commented Jan 13, 2020

Cheddam commented Jan 13, 2020

lekoala commented Jan 13, 2020

Cheddam commented Jan 15, 2020

maxime-rainville left a comment

emteknetnz commented Jul 2, 2020

lekoala commented May 12, 2021

GuySartorelli commented Jul 1, 2022

GuySartorelli commented Jul 7, 2022 •

edited

GuySartorelli commented Jul 7, 2022

emteknetnz Jul 11, 2022

GuySartorelli Jul 11, 2022

emteknetnz Jul 11, 2022

GuySartorelli Jul 11, 2022

emteknetnz Jul 11, 2022

GuySartorelli Jul 11, 2022

emteknetnz Jul 11, 2022

GuySartorelli Jul 11, 2022

emteknetnz left a comment •

edited

GuySartorelli commented Jul 11, 2022

emteknetnz commented Jul 11, 2022 •

edited

emteknetnz left a comment

GuySartorelli commented Jul 25, 2022

emteknetnz left a comment

	protected $searchField = null;
	protected ?string $searchField = null;

NEW: Allow search field customisation #9341

NEW: Allow search field customisation #9341

Conversation

unclecheese commented Nov 26, 2019 • edited by GuySartorelli

Notes

Parent issue

maxime-rainville left a comment

Choose a reason for hiding this comment

unclecheese commented Nov 26, 2019

maxime-rainville commented Nov 27, 2019 • edited

unclecheese commented Jan 13, 2020

Cheddam commented Jan 13, 2020

lekoala commented Jan 13, 2020

Cheddam commented Jan 15, 2020

maxime-rainville left a comment

Choose a reason for hiding this comment

emteknetnz commented Jul 2, 2020

lekoala commented May 12, 2021

GuySartorelli commented Jul 1, 2022

GuySartorelli commented Jul 7, 2022 • edited

GuySartorelli commented Jul 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emteknetnz left a comment • edited

Choose a reason for hiding this comment

GuySartorelli commented Jul 11, 2022

emteknetnz commented Jul 11, 2022 • edited

emteknetnz left a comment

Choose a reason for hiding this comment

GuySartorelli commented Jul 25, 2022

emteknetnz left a comment

Choose a reason for hiding this comment

unclecheese commented Nov 26, 2019 •

edited by GuySartorelli

maxime-rainville commented Nov 27, 2019 •

edited

GuySartorelli commented Jul 7, 2022 •

edited

emteknetnz left a comment •

edited

emteknetnz commented Jul 11, 2022 •

edited