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

CiviMail - New widget for selecting recipients #26522

Closed
wants to merge 5 commits into from

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jun 13, 2023

Overview

When composing a mailing, this replaces the single include/exclude/groups/mailings widget with two widgets, one for include and another for exclude. Groups and mailing are still combined together in autocomplete results, but are loaded more efficiently by APIv4.

Before

Single widget for include/exclude groups and mailings. Loading can be quite slow because it has to load 4 times.

image

After

Two widgets, each one loads only once so it's much faster:

image

When building the mailing from search results, the hidden group is simply called "Search Results" and cannot be de-selected:
image

Technical Details

Builds on #26507 to autocomplete a UNION of both groups and mailings.

Comments

Technically, APIv4 is capable of doing enough UNIONs to show each group and mailing twice (for both include and exclude in the same widget). But I personally don't like that UX - I find it cluttered when the same results pop up twice, you end up scrolling past more things you don't want.

colemanw added 2 commits June 11, 2023 16:35
Supports UNION and UNION ALL with global limiting/sorting/having. Per SQL UNION rules,
names do not need to match (columns in the UNIONed tables will be aliased to
match the column names of the first table) but the number of columns and the data
types do need to match.
@civibot
Copy link

civibot bot commented Jun 13, 2023

(Standard links)

@civibot civibot bot added the master label Jun 13, 2023
@colemanw colemanw changed the title Crm mailing recipients4 CiviMail - New widget for selecting recipients Jun 13, 2023
colemanw added 3 commits June 13, 2023 13:28
Generally improves handling of Autocomplete searchDisplays that use field aliases and unions
This uses an autocomplete callback with UNIONs to select from both groups and mailings,
but instead of a single widget for incluce/exclude it provides two widgets for better usability.
@colemanw colemanw force-pushed the crmMailingRecipients4 branch from 8c08ff2 to 8f8c874 Compare June 13, 2023 17:28
@larssandergreen
Copy link
Contributor

Will be great to have a speedier interface for this. Agreed that include and exclude are better in two fields.

I think for the UI, those narrow fields are going to be pretty painful to use (especially if you have lengthy names for mailings). Maybe one per line instead?

I don't think the "Group by Contact" / "Parent Group by Contact" are useful info and they just make it harder to use by showing fewer groups, suggest removing. The icon is enough to tell you if a group is smart or not.

I like the Unsent/Sent Mailing thing, but would prefer a created or sent date to the name of the creator. Not sure what the third line in there is, seems to be a duplicate of the first one. Order should be in desc by date (created or scheduled/sent), not clear what it is now.

Do we include archived mailings? They are not included in the current widget. I'd say no.

It seems like I can sometimes include and exclude the same group and sometimes cannot. Can't seem to find a rhyme or reason as to when. When I can include and exclude, I can also include or exclude twice. This clears up one save and re-open, but potentially confusing for the user. Good that I can't include or exclude the current mailing.

Not sure if it is related, but I'm getting a more than usual amount of "Sorry an error occurred and your information was not saved" when testing this on a Traditional Mailing (not seeing any on Mosaico).

@colemanw
Copy link
Member Author

@larssandergreen I wholeheartedly agree they ought to be put on two lines but unfortunately the angular template is quite brittle and I was unable to do it in a way that was friendly toward Mosaico (which reuses the same template). I was hoping to avoid having to coordinate a syncronized release of Mosaico & core. I'll keep thinking about the smoothest path there, but for now this works for both.

Do we include archived mailings? They are not included in the current widget. I'd say no.

Yes, the current widget does include them. Debatably it should not, but I didn't want to change too many things at once.

Not sure what the third line in there is, seems to be a duplicate of the first one.

Can you share a screenshot?

Order should be in desc by date (created or scheduled/sent), not clear what it is now.

Currently it's ordered by:

  1. Groups before mailings
  2. Name ASC

Sorting by name is pretty standard for an autocomplete, but I see the argument for date.

@awestuk
Copy link
Contributor

awestuk commented Jun 15, 2023

Tested this and it's a big improvement! The two dropdowns are faster, and the extra information is useful. It feels much more solid, and all the flakiness of the previous field is gone.

Agree that archived mailings probably shouldn't be in there, but if they were before then fair enough. It'd be nice to have a small delay before the search kicks in to avoid extra ajax calls.

(Just as a matter of interest, does the dropdown have to make an ajax call when searching? I only ask because searching/adding multiple groups to the 'In Groups' dropdown in Search Kit is instantaneous, even with our 5k groups.)

@colemanw
Copy link
Member Author

Replaced by #26585

@colemanw colemanw closed this Jun 21, 2023
@colemanw
Copy link
Member Author

@larssandergreen thanks for testing, I've opened up a new PR #26585 to incorporate the suggestion to split the include/exclude fields on 2 lines.
@awestuk to answer your question, I'm pretty sure the Recipients widget was originally written without ajax, but the javascript started to choke on loading thousands of groups plus thousands of mailings in the same dropdown.

@colemanw colemanw deleted the crmMailingRecipients4 branch July 10, 2023 16:18
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