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

[draft] Riverlea stream UI (listing, previewing, editing) #32344

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ufundo
Copy link
Contributor

@ufundo ufundo commented Mar 11, 2025

Overview

This builds on #32127 to add a UI for previewing and editing Riverlea streams.

It's not ready for merging but the idea is to demonstrate the point of adding the entity in #32127 and get feedback on a) how it looks; b) the technical approach.

Before

  • streams are hard coded things

After

  • streams are db entities
  • listing of streams at civicrm/admin/riverlea/streams
  • base streams are not editable
  • you can clone the base streams or add a blank stream in order to edit it

Technical Details

It's WebComponents again. This seems quite a good place to test them because a) there's opportunity for various bits of interaction between the components in this PR; but b) there probably quite self-contained and won't need to interact with anything outside of this PR.

The stream list also component also I think demonstrates how a WebComponent search kit listing might work.

Comments

@vingle would you be able to take a look? There's no upgrader for the entity included here so on the test site is probably best.

Copy link

civibot bot commented Mar 11, 2025

🤖 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 Mar 11, 2025
@ufundo ufundo force-pushed the riverlea-stream-list branch 2 times, most recently from 2390700 to 6674d53 Compare March 11, 2025 23:55
@ufundo ufundo force-pushed the riverlea-stream-list branch from 6674d53 to a2400a1 Compare March 12, 2025 08:55
@ufundo ufundo changed the title [draft] Riverlea stream editing [draft] Riverlea stream UI (listing, previewing, editing) Mar 12, 2025
@ufundo ufundo force-pushed the riverlea-stream-list branch from a2400a1 to d9b35c9 Compare March 12, 2025 09:22
@ufundo
Copy link
Contributor Author

ufundo commented Mar 12, 2025

Ok, there was some teething on Drupal (and the list is a bit squished).

But demo should be working now here: http://core-32344-987dd.test-1.civicrm.org:8003/civicrm/admin/riverlea/streams

@vingle
Copy link
Contributor

vingle commented Mar 12, 2025

This is really impressive @ufundo. The one-click preview alone is huge!

I've only time for a quick explore this week. I'd perhaps separate test / discussion around the follow areas:

  • the code itself, which I can't review. Is the stream stored in the dbse as JSON? And is that what's ouptuted in the 'details'?
  • the UX/UI, which I can… I'd be tempted for more of an extensions manager list+details+buttons table-ish list UI than two-column grid. Could still be a web component for each table row (?) as it's nice you're using that. We want something that both survives all changes, but also reflects them to help illustrate, a nice design challenge. Happy to have a go on a branch…
  • what can be customised… (I like your choices but would prob add primary hover, unless we swap that for a tint - and the backgrounds are a bit misleading without simplifying how RL core handles backgrounds). I'm assuming 'new streams' are built on core variables, ie Minetta?
  • how this interacts with dark mode… I love that you can edit dark mode colours.. ie if I clone Hackney I get Hackney's Dark mode? And if I make a new stream I get none? Or Minetta's? Obvs is theme settings is forcing light mode, this doesn't fully work - might need some text around that.
  • test how this interacts with all the CMSs and most popular admin themes
  • add some 'test this before you make it public text' to the enable on front-end pages button. Also I really need a RiverLea 'crm-buttons' flex wrapper to add gap to button groups!

And then the big questions/tests

  • what happens to custom streams when underlying RL core or RL streams change?
  • can people enter custom css or values that break anything. Or that force something that requires RiverLea to not change in the future without breaking (e.g. it locks font-size to rems and colour to rgba which sounds ok?)
  • is the cascade order of the custom stream consistent across all CMSs front and back-end? Triggered by a recent Drupal 10.4/11.1 upgrade that changed the cascade order between custom themes and contributed themes and broke a bunch of sites…
  • if someone sets text and bg to white, or font-size to 0 or adds .crm-container {display: none'} is there a safe exit/reset mode and does that always work?
  • are the permissions to limit access to this screen sufficient / absolute? I think the worst case security risk is changing form labels with css to get people to enter, say, passwords or credit cards in plain text fields.. maybe more of an issue if this is to be used in combination with the iframe/oembed.

@ufundo ufundo added the run-standalone Civibot should setup demos+tests for Standalone label Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master run-standalone Civibot should setup demos+tests for Standalone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants