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 - Add default text/image to displays; support custom file fields #21992

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Nov 6, 2021

Overview

Allows for "placeholder" text in a search display when a column value is empty. Also works for image fields.
Image fields now support custom file fields to display images out-of-the-box.

Technical Details

This breaks out image into its own column type, and generates the url to access custom file fields.
A new 'empty_value' property allows for a placeholder text/image to display when a field is blank.

@civibot
Copy link

civibot bot commented Nov 6, 2021

(Standard links)

@civibot civibot bot added the master label Nov 6, 2021
@colemanw colemanw force-pushed the searchKitImageAndDefault branch from 50c55ab to fc9297e Compare November 7, 2021 19:12
@colemanw
Copy link
Member Author

colemanw commented Nov 7, 2021

retest this please

…ields

This breaks out image into its own column type,
and generates the url to access custom file fields.
A new 'empty_value' property allows for a placeholder text/image to display when a field is blank.
@colemanw colemanw force-pushed the searchKitImageAndDefault branch from fc9297e to cc7246d Compare November 8, 2021 04:39
@jaapjansma
Copy link
Contributor

test this please

@jaapjansma
Copy link
Contributor

We (@kainuk, @BettyDolfing and I) are reviewing PRs and we checked this PR.

  • General standards
    • Explain (r-explain)
      • PASS : The goal/problem/solution have been adequately explained in the PR. This could have been explained better it tooks some guesses and what to do. We missed a step a step by instruction on how to test or reproduce this.
    • User impact (r-user)
      • PASS: The change would noticeably impact the user-experience (eg requiring retraining), however we believe it is a naturally approach.
    • Documentation (r-doc)
    • Run it (r-run)
      • PASS: We created a new search with search kit and included a custom file field and displayed this as an image. We also provided a default image for when the field was empty. We then checked the output of the display and it all seemed to work.
  • Developer standards
    • Technical impact (r-tech)
      • PASS: The change potentially affects compatibility, but the risks have been sufficiently managed.
    • Code quality (r-code)
      • PASS: The functionality, purpose, and style of the code seems clear+sensible.
    • Maintainability (r-maint)
      • PASS: The change sufficiently improves test coverage.
    • Test results (r-test)
      • PASS: The test results are all-clear.

@colemanw can you also update the user documentation?

Although we miss the update for the user documentation we believe this PR is ready to be merged. @demeritcowboy what do you think?

@jaapjansma jaapjansma added needs-documentation merge ready PR will be merged after a few days if there are no objections labels Nov 17, 2021
@demeritcowboy demeritcowboy merged commit 16c309a into civicrm:master Nov 17, 2021
@colemanw colemanw deleted the searchKitImageAndDefault branch November 17, 2021 15:39
@colemanw
Copy link
Member Author

Thanks for the review @jaapjansma!
I will try to write some docs when I get time, but right now I'm focused on building out features for SearchKit.
It would be nice to organize a docs sprint just for SearchKit now that people are really using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-test master merge ready PR will be merged after a few days if there are no objections needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants