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

[REF] SearchKit - display code refactor + pager options #21049

Merged
merged 4 commits into from
Aug 9, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Aug 6, 2021

Overview

Code refactor to improve code reusability in SearchKit displays, plus a couple new options for configuring the pager:

image

Technical Details

My goal is to merge two divergent chunks of code: SearchKit has 2 results tables, one is the table shown when an admin is composing a search, and the other is a SearchDisplay of type table. They are very similar in appearance, but were written for different use-cases. Notably, the table on the admin screen lets you add, remove and move columns. I also wrote it first. Had I written the SearchDisplay code first, I would have adapted it for use on the admin screen instead of writing the other from scratch.

I'm trying to fix that design error and adapt the table display for use on the admin screen and ditch all that single-use code, but it's turned into a massive undertaking so here's a preliminary PR with the refactoring I've done so far. It's mostly just moving code around, the only visible change is to improve the pager on SearchDisplays to have feature-parity with the one on the admin screen ("adjustable page size" and "show count").

@civibot
Copy link

civibot bot commented Aug 6, 2021

(Standard links)

@civibot civibot bot added the master label Aug 6, 2021
@colemanw colemanw force-pushed the searchDisplaysRef branch 2 times, most recently from 82db42a to 316baea Compare August 8, 2021 16:23
@seamuslee001
Copy link
Contributor

I'm going to merge this based on @MegaphoneJon 's comments in mattermost https://chat.civicrm.org/civicrm/pl/nkyt195z5bfhmd5mscb94sbt5y

@seamuslee001 seamuslee001 merged commit c794e5e into civicrm:master Aug 9, 2021
@seamuslee001 seamuslee001 deleted the searchDisplaysRef branch August 9, 2021 08:40
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.

2 participants