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

ContactSummary - Replace Relationships tab with SearchKit display #27701

Merged
merged 12 commits into from
Oct 4, 2023

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Oct 2, 2023

Overview

2nd tab now replaced by a search display (first was Notes).

Before

Outdated code to render 2 tables: one for active relationships and one for inactive ones.

After

Strikingly similar 2-table layout, but uses SearchKit.

Comments

Frustratingly I wasn't able to delete very much of the old code because it's still in use by the obscure "User Dashboard" that most people have never heard of. It would be good to replace that whole thing with SearchKit too.

@civibot
Copy link

civibot bot commented Oct 2, 2023

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Oct 2, 2023
@colemanw colemanw force-pushed the relTab branch 2 times, most recently from d5cd780 to 96224b5 Compare October 2, 2023 15:01
@colemanw colemanw marked this pull request as ready for review October 2, 2023 15:22
@colemanw colemanw changed the title Rel tab ContactSummary - Replace Relationships tab with SearchKit display Oct 2, 2023
@aydun
Copy link
Contributor

aydun commented Oct 3, 2023

Relationships like 'Child of' 'Sibling of' have a 'Manage Case' link in the actions menu despite their being no related cases.
image

One thing with rows formatted as 'disabled' is that all the action links are also light grey suggesting that those actions are disabled when they're not. Maybe leave the formatting of actions alone. Not specific to this display.

Just noting that this replaces the display in core, not as part of AdminUI or SearchUI that can be disabled. I think that's ok for this display but if anyone has customised this tab it will force them to change with whichever release contains this. It would be useful to document how this can be overridden (I think that is create your own tab, and disable this one by going to Formbuilder and unticking the 'Add as tab' option).

@colemanw
Copy link
Member Author

colemanw commented Oct 3, 2023

@aydun thanks for testing. Oof that looks like a bug in the way the extra join provider works. Yay another rabbit hole blocking SK UI swaps. Down I go...

@colemanw
Copy link
Member Author

colemanw commented Oct 4, 2023

@aydun fixed!
At this point this PR is about 25% SearchDisplay and 75% fixes and unit tests. So IMO it would be a good thing to merge before branching master tomorrow.

@aydun
Copy link
Contributor

aydun commented Oct 4, 2023

Nice - fixes the issue above.

@aydun aydun merged commit 1bbb5d4 into civicrm:master Oct 4, 2023
@aydun
Copy link
Contributor

aydun commented Oct 4, 2023

... and good to see we didn't lose you down the rabbit hole for too long!

@colemanw colemanw deleted the relTab branch October 4, 2023 16:52
@colemanw
Copy link
Member Author

colemanw commented Oct 4, 2023

Thanks @aydun good to be back out of rabbit-land, however briefly.

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