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 - Merge admin results table with searchDisplay code #21069

Merged
merged 2 commits into from
Aug 10, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Aug 9, 2021

Overview

This improves and streamlines the SearchKit Angular code to reconcile 2 different ways of fetching, formatting & displaying results. This reduces code duplication, while adding a few features from the admin table to all search display tables.

Before

The SearchDisplay subsystem has its own way of fetching, formatting and displaying a table of results, which looked nearly identical to the table on the SearchKit admin screen, but used completely different code. This redundancy was starting to cause maintenance headaches.

After

Consolidated all search results to use SearchDisplay code. 2 features on the admin table that were missing from SearchDisplays (a search button and adjustable page size) have been added. One "extra" feature from the admin table (the "Search/Auto" toggle) has been dropped; it may have been more confusing than helpful.

Technical Details

This greatly simplifies the SearchKit admin code by handing off the table to a specialized searchDisplay (adapted from crmSearchDisplayTable). Builds on the refactoring done in #21049

Before / After Comparison

Despite having all its code ripped out, the admin table looks almost identical:

Before After
image image

@civibot
Copy link

civibot bot commented Aug 9, 2021

(Standard links)

@civibot civibot bot added the master label Aug 9, 2021
@seamuslee001 seamuslee001 self-assigned this Aug 9, 2021
This allows search displays to present a button to the user instead of running immediately
This greatly simplifies the SearchKit admin code by creating a
specialized searchDisplay (copied from crmSearchDisplayTable) which
eliminates all the code for the admin screen to fetch, format and display results.
@colemanw
Copy link
Member Author

colemanw commented Aug 9, 2021

I've rebased this now that #21049 has been merged.

@eileenmcnaughton
Copy link
Contributor

I gave this a light r-run & could not detect any differences - which is a good thing. I think merging this now (fairly early in the cycle) is good as it should give us time to find anything....

@eileenmcnaughton eileenmcnaughton merged commit a933031 into civicrm:master Aug 10, 2021
@eileenmcnaughton eileenmcnaughton deleted the searchKitAdminResultsTable branch August 10, 2021 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants