Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SearchKit - Use a search display to display searches #21270

Merged
merged 3 commits into from
Sep 1, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Aug 26, 2021

Overview

This updates SearchKit to use a search display to display the list of searches.

⚠️ WARNING: may cause brain recursion 🤯

Before

The table of saved searches was just an ad-hoc thing I wrote in Angular. It doesn't have a pager so wouldn't scale past a few hundred.

Screenshot from 2021-08-26 11-51-14

After

Now it uses a search display, which reduces code while adding a few nice features like a pager and in-place edit:

Screenshot from 2021-08-26 11-43-33

Technical Details

Most of it I was able to do with vanilla searchDisplay code. In order to render the fancy custom-coded columns like the list of Afforms, I added a new column type which lets you supply your own angular template:

Screenshot from 2021-08-26 12-01-57

@civibot
Copy link

civibot bot commented Aug 26, 2021

(Standard links)

@civibot civibot bot added the master label Aug 26, 2021
@@ -189,7 +189,7 @@ public static function &fields() {
'name' => [
'name' => 'name',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('Saved Search Name'),
'title' => ts('Name'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought where we landed was that the title would assume low-context and <html<label> would assume high context - so instead of reducing the information in the title we would add a html label tag

@eileenmcnaughton
Copy link
Contributor

This passed r-run - I'm just not sure the schema changes is what we agreed on?

@eileenmcnaughton
Copy link
Contributor

@colemanw this is stale - I'm assuming the dao change is now otherwise merged

@eileenmcnaughton
Copy link
Contributor

Looks like my concern has been addressed - MOP

@colemanw
Copy link
Member Author

Thanks @eileenmcnaughton - while re-testing this I found another bug: #21333

@colemanw colemanw merged commit a7a5556 into civicrm:master Sep 1, 2021
@colemanw colemanw deleted the searchKitListing branch September 1, 2021 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants