Problem/Motivation
The ability to select a contact category form is missing from the site wide contact form.
To reproduce:
- Install standard profile
- Add an additional contact category
- View /contact
What the will look like
What's missing
Proposed resolution
Introduce a block with a configurable list of contact forms
Remaining tasks
Update help text according UX review
Discuss enabling the Contact forms block automatically
Add tests
User interface changes
New block with a configurable set of links to other contact forms
API changes
no
Depends on the issue
#1477802: Shorter description for contact category weight
#599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page
Originally was:
Suggests to use tabs #1849158: Expose contact category-specific forms and/or their URLs somewhere
Comment | File | Size | Author |
---|---|---|---|
#131 | interdiff_127-131.txt | 1.99 KB | ranjith_kumar_k_u |
#131 | 1997692-131.patch | 11.99 KB | ranjith_kumar_k_u |
| |||
#130 | 1997692-nr-bot.txt | 3.55 KB | needs-review-queue-bot |
#127 | 1997692-127.patch | 11.99 KB | smustgrave |
#127 | 1997692-127-tests-only.patch | 2.05 KB | smustgrave |
Comments
Comment #1
alexpottHere's a patch
Comment #3
alexpott#1: 1997692.1.contact-categories.patch queued for re-testing.
Comment #4
BerdirThis is a problem for the NG conversion as entities with bundles need the bunde from the beginning, otherwise we can't fetch the correct field definitions.
Can we maybe default to the default category (or first entry if not set) and provide some sort of flag to the form to display the category selection or not? Maybe a different form mode?
Because I don't think this works anyway? For example on form rebuild, $message would have a category?
Comment #5
alexpottHmmm.... ok we need to re-think the site wide form then... allowing people to create categories but then giving sitebuilders no functionality to pick between other than creating a custom block or menu with links to each contact form for each category seems like quite a big regression.
This code is not new to this patch :)
Comment #6
alexpottSo digging into this some more... this seems to be a dupe of #1849158: Expose contact category-specific forms and/or their URLs somewhere BUT the image on that issue is completely misleading as the menu tabs are not there!
In order to providing ordering and default site wide contact form category selection, convert all this routes we need this issue to be solved as how do we know that the conversions are working? There are no tests for how contact categories are exposed or ordered...
Also what the Drupal 7 functionality has that we seem to have missed is that is was possible to configure the contact form with no default category and then the drop down category selector would have a "Please select" option added... forcing people to make a choice. I think this type of interaction can be important when you have few contact categories and you want people to actually think before submit a contact form...
Comment #7
andypostAs I mentioned in #1849158: Expose contact category-specific forms and/or their URLs somewhere better expose a kind of jump menu. Anyway we need to reload page when changing category because of different fields on each category
Comment #8
andypostThought about a bit more... suggested approach is wrong - each category have different set of fields so page re-load is needed anyway.
So if no default category we can simply theme('item_list') while tabs approach from #1849158: Expose contact category-specific forms and/or their URLs somewhere rejected because site can have a lot of categories.
Also postponing #1477802: Shorter description for contact category weight about help text
EDIT added related and dependant issues to summary
Comment #9
Bojhan CreditAttribution: Bojhan commentedNot sure what feedback is expected here
Comment #11
dcam CreditAttribution: dcam commentedhttp://drupal.org/node/1427826 contains instructions for updating the issue summary with the summary template.
Comment #12
mandarmbhagwat78 CreditAttribution: mandarmbhagwat78 commentedIs it really required to have category in the contact form?
Contact module is already providing a way to call different contact category forms.
Comment #13
Gábor HojtsyYeah I agree with @andypost in #8 that different categories can now have different fields in Drupal 8, so you cannot just pick a category, you need to reload the page. So a categories menu or tabs on the contact forms or something along those lines should be used. So sounds like a duplicate of #1849158: Expose contact category-specific forms and/or their URLs somewhere?
Comment #14
andypostThis really needs usability review because holds #599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page
Comment #15
yoroy CreditAttribution: yoroy commentedSo the ux question is how to provide category selection now that we need to do a page load when a category is chosen?
Comment #16
andypost@yoroy Yes, because each contact form could have own set of fields
Comment #17
catchiirc this was a known regression when the original patch went in. It's possible for site admins to make menu links to the forms or expose this in some other way so it didn't seem worth blocking the patch (or indeed release). Downgrading to normal since the feature was deliberately removed.
If we can find a nice way to to do this in core I'd be fine adding it back, but we don't provide visitor-facing mechanisms for navigating to all nodes either.
Comment #18
BerdirOn the create form, we could have something like "[ ] Create a menu link to this contact category". Or if we'd generalize the menu thing on nodes with an menu link entity reference, we could just use that here as well.
Or, something else that I've been thinking is that the main URI of a contact category shouldn't be the edit form like all other config entities but contact/$category. Unlike a image style or action, they actually have a publicly displayed page. The only problem is then the personal contact category, which doesn't have a static URI. Then you could simply click on the category on the overview and it's easier to find.
And all of that is actually #1849158: Expose contact category-specific forms and/or their URLs somewhere, so we might not even need this issue?
Comment #19
andypostAnother way is implement a configurable block to allow choose categories so allowing to have navigation element specific to category set
Comment #20
heather CreditAttribution: heather commentedWhen testing the contact form, with the new "Form modes" I expected to be able to expose a specific "form mode "somewhere. Such as making a "mini" contact form, and placing into the sidebar. I looked into the Custom blocks library expecting to be able to find it there.
I noticed that my categories created their own URLs.
E.g., example.com/contact/feedback or example.com/contact/help
So, I also wondered if the form modes would allow me to specify the URL of a specific form mode, or if these paths could be assigned through the menu system.
I think this is something that would be worth testing out.
In this issue, they proposed tabs: #1849158: Expose contact category-specific forms and/or their URLs somewhere
That is interesting, since you could assign form modes for example, into the menu system and have "menu tab" entries as we do with views.
I don't know if we want to bring Views into the display of a contact form, but the pattern of controlling tabs with the menu system is consistent at least.
I think a drop down menu does make sense in some cases, but not all.
Comment #20.0
heather CreditAttribution: heather commentedadded dependent and related issues
Comment #21
andypostMaybe better to provide a configurable block to display a list of related categories so this will allow users to place this block contextually
Comment #22
pwolanin CreditAttribution: pwolanin commentedSo, is there any consensus on a solution? Is a block the next step?
Comment #23
andypostLet's start from block as easiest and most configurable aproach
block settings
block placed
Comment #24
andypostAnother round and re-roll
Added ordering of categories and implemented current cache render
Comment #25
andypostinterdiff
Comment #26
sunHm. When I read "block", I thought of 1 block per category, exposing the contact form itself. @heather also mentioned that as a concrete use-case. But I guess that's a slightly different purpose, and thus probably different issue.
I'm not sure I can see the use-case of a block that shows links to all (or selected) contact forms. — But in any case, that's a list, and lists are the domain of Views. So instead of implementing a custom block, we should ensure that the contact categories are exposed to Views. That way, you can simply build that block/view yourself, if you happen to have that use-case.
Aside from these points, there actually hasn't been much feedback on the claimed regression here yet. My impression is that we've jumped on the technical/development path very early, without validating the use-case.
The original issue removed the category selector in favor of resolving the much more common + desired use-case for having category-specific input fields on each contact form. At least my experience on 100% of real world sites that are using Contact module with multiple categories (and which didn't resort to Webform module due to Contact module's limitations in D7 yet) is that the former category selector functionality worked against the site owner:
By just taking the most common categories, Sales/Support/Other, it's immediately apparent that each form needs custom fields, and on top, it doesn't make sense to funnel all possible contact requests through a single contact page. The effective result of doing that is that you receive plenty of wrongly categorized contact requests, which is a general sign that end-users were not guided into the right contact channel (upfront).
Just to continue the example set of categories (which is just one example of many possible), the UX + funnel problem is (typically) resolved by placing a "Contact Sales" link somewhere in the header, a "Get Support" link somewhere in the footer, and a link to a general/other contact form somewhere deeper in the "About" section. That ensures the right discovery/entry paths for end-users, without a confusing "category" selector deeper in the funnel.
So IMO, the usability issue is about "making it easy to place a link to a particular contact form" only.
Since I'm generally skeptical of all the in-place menu link creation/editing interfaces (as they're lacking essential context that is required for editing a list/tree of menu links), I'd rather consider that a general weakness of the Menu module UI though — the menu UI should allow you to pick an entity type + autocomplete/select an entity that you want to link to. Thus, you'd choose "Contact category" and select the "SomeCategory form" to place a link to that form, without having to know its internal/system path.
Comment #27
seantwalshOk would like to help with this and other Contact Form issues. After reading through I believe the following should probably happen and will roll patches accordingly.
Comments welcome!
Comment #28
seantwalshAdmin UI - I've linked to the specific "category" via the label.
Comment #30
KayGridley CreditAttribution: KayGridley commentedCurrently working on a issue summary.
Comment #31
andypostThe related issue is blocked on that, so can we get conclusion here?
Comment #32
jhodgdonVast sections of the Core contact module's UI are completely confusing and useless if this functionality is not working, so I think this is more of a "Major" issue. Either the UI for categories needs to be removed, or this needs to be fixed.
Comment #33
Berdir#27.3 is spot on.. What we have now are contact *pages* and not categories. Would be nice to rename to that, but that would be quite a change that we would need to be signed off by the core maintainers.
#27.2 is also very useful, +1 to that.
I'm not really sure how the block is helpful, didn't look at it yet but can't you just create a menu with links to the various contact pages? We could even define a new default menu with defaut menu links for each category, then users can easily customize it to show the ones they want? Or a "Create link" entity operation that makes it easy to create a new menu link to category page?
If you want to have a category selector then add a list field or taxonomy term reference to the contact page. The only problem that core currently can't do to replace the old behavior with that is having different e-mail recipients based on the selected category.
We do have usabilities issues/bugs here, but I don't see this as a major regression or anything being useless here.
Instead of customizable URL's, it would be easier to add path aliases for a page, if you want to show it somewhere else.
Comment #34
Gábor HojtsyThe create link operation sounds confusing. Eg. when you create a view, the menu item creation is part of the process. For contact categories, it would be a separate operation but an integrated action link.
Comment #35
andypostSuppose absence of consensus here caused by different usage of contact module.
IMO d7 version of contact is useless and confusing because most of sites I've seen or build are used webform or entityform modules to provide "feedback" or to gather data from user.
sun's idea and implementation #1825044: Turn contact form submissions into full-blown Contact Message entities, without storage in d8 actually brings contact module to new era and makes it a great starting point for this kind of functionality.
Also #1849158: Expose contact category-specific forms and/or their URLs somewhere gives ability to have each form own URL, so now we just need a way to link this pages from other site's pages.
Menu links does not help IMO because mostly I need to provide a set of contact links that are different for different pages that's why #23 approach allows this blocks to be reused for different pages (and suppose panels)
Comment #36
pwolanin CreditAttribution: pwolanin commentedI agree with crowdcg and andypost that having the block available will make it easy to set this up for sites and makes it rather more obvious what's going on and it also simple enough to implement here that it's reasonable for core.
While you could achieve the same effect with menu links, I don't think it's useful to e.g. make this depend on menu_ui module.
Comment #37
andypostfiled #2285083: Rename contact category to contact form
Comment #38
seantwalshWorking on an updated patch for this today.
Comment #39
seantwalshStill probably going to fail, but now at least it will apply after the PSR4 change. Also, made a change to not apply a link the "Personal contact form" as this posed more challenges than were worth the benefit.
Comment #40
seantwalshComment #42
seantwalshOk, this should pass now. Thanks to @pwolanin for pointing me in the right direction!
Comment #43
pwolanin CreditAttribution: pwolanin commentedFor inline comments like:
Should the variable name come before or after the type? I see examples of both in core I think.
Comment #44
andypostBy default
@var data-type var-name
Comment #45
seantwalshAdjusted the inline comments.
Comment #46
andypostSo only tests left here, and #2285083: Rename contact category to contact form
Comment #47
andypostComment #48
BerdirAnother point re category vs. page.
We're missing just one feature to be able to replace the 7.x-style category selection with a list or taxonomy reference field, and that is the ability to change the e-mail address by category. If we could find a way to support that, then we could very easily migrate the 7.x contact categories to a single page with a field. Then switching the category would just work as we don't need to do crazy stuff like switching the bundle/displayed fields.
Maybe a custom field type that can store a label and recipient(s)?
Comment #49
larowlanI agree with this comment
Comment #50
chx CreditAttribution: chx commented> we should ensure that the contact categories are exposed to Views.
You can already build Views querying pretty much anything in CMI http://cgit.drupalcode.org/sandbox-chx-1857558/log/?h=viewsception refactoring this into the D8 version of efq_views is a trivial exercise (which I will do in the next few months).
Comment #51
mgiffordComment #54
rpayanmhere the reroll.
Comment #56
rpayanmComment #57
BerdirShould we instead adjust l() to accept a Url, so it is the same as e.g. \Drupal::l() ?
Comment #59
yoroy CreditAttribution: yoroy commentedThanks for the reroll! A screenshot of what's being changed/added would be helpful, I'll review for UX.
Comment #60
rpayanmComment #62
rpayanmComment #63
yoroy CreditAttribution: yoroy commentedA quick look with simplytest.me shows that the table listing the categories is broken:
Also, I don't see a category selector on the contact form but maybe that's intentional?So yes, there's a block listed for the contact categories on /admin/structure/block, but the link doesn't work, I don't get a modal window for its configuration to show up.Comment #64
rpayanm@yoroy The patch is not yet complete.
Comment #66
yoroy CreditAttribution: yoroy commentedAh, ok, I'll wait a bit then. Thanks.
Comment #67
andypostFixed patch, ready for UI review
Comment #68
rpayanmminor improve :)
@yoroy ready for the action ;)
Comment #69
andypostre-roll after #2351847: Rename getCacheTag() to getCacheTags()
Still needs UX approval and then tests
Comment #70
yoroy CreditAttribution: yoroy commentedOk, had a first look at things. First off, I think the general approach is a useful reinterpretation of having contact form categories (yay for the renaming of categories to forms btw). We do get yet another place where you create "content" for a block which you then have to discover somewhere to show it else but that's a systemic issue.
1. Very good to have the titles linked to the actual forms.
2. Lets rename the 'Selected' column to 'Default' because that's the wording used on the checkbox that sets this.
3. The help text needs to be updated to reflect this new implementation
Current help text:
First stab at rewriting. The first sentence could be shortened:
Create seperate contact forms for different purposes.
Then, with the new contact menu block, do we still need the single 'Contact' menu item in the footer menu?
The last bit is a rather specific site building suggestion. It is important though to have a link to the blocks page so people may find the new Contact forms block. Maybe:
Each form gets a link in the [link]Contact forms menu[/link]. You can enable this menu on the [link]Block layout[/link] page.
---
With the menu enabled, the menu links seem to be missing an active state when viewing one of the linked contact forms
---
Probably unrelated to this patch, but it's weird to edit the provided default 'Website feedback' form and see a required field without a value.
Comment #71
larowlanMarked #2359857: Link each row in contact form list controller to the form as duplicate of this. ++
See #2349651: Default contact form does not send email as email recipient is not set during the installation.
This will need to be done in a cache-safe manner, possibly #post_render_cache
$entity->link() would be simpler
couldn't we use #type => 'tableselect' here instead?
again $entity->link() would be enough here
Comment #72
larowlanYes that can go, because that only happens in standard.profile
Comment #73
larowlanFixes everything from the reviews except tests and questions about tableselect - left as is for now
Comment #74
andypostAbout default/selected that was discussed in #599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page
@yoroy the pointed issue is another UX one, that blocked by that.
Comment #75
yoroy CreditAttribution: yoroy commentedWhat we have in D7:
What we have in D8
What this patch does
About this solution
Creating multiple forms was already possible in D8. What this patch does is enable discovery of and linking to each individual form, which was not yet possible (because of just one generic ‘Contact’ link)
Ability to configure different contact forms with customised fields etc. is a nice feature. We still had to fix making those forms findable. Adding a ‘contact forms’ menu block for this seems right.
Review of the latest patch
/admin/structure/contact
/admin/structure/contact/manage/feedback/form-display
Other, unrelated
Needs work
Comment #76
yoroy CreditAttribution: yoroy commentedThe duplicating of UI elements is tackled in #2384545: $element['#ajax']['callback'] is broken, hence breaking e.g. inserting images in CKEditor
Comment #77
jhodgdonWe have an open issue about updating the main help page for the contact module, which is the top part of hook_help(). Whether #2091395: Update hook_help for Contact module or this issue gets done first, this patch does need to update the main module help page to describe the new functionality being added in this patch. This patch is currently only updating the help on route 'contact.form_list' (the page-level help on the admin page for contact forms); it is not updating the main help text, and it needs to do that.
Meanwhile I am about to make a patch for #2091395: Update hook_help for Contact module to at least get the help there updated to the current functionality of the Contact module (without this patch, since I am not certain this patch will be done, it looks like it's not all that active and it is a bit buggy as of the last review by yoroy).
Also, as a minor note on the patch, in the hook_help() in contact.module:
That previous line needs to be inside the if( module exists 'block' ) test as well.
Comment #78
andypostGood question for separate issue OTOH do we need to add newly created forms to all placed blocks?
There's nothing to edit there, but probably ability to change name makes sense (changing machine name should be always disabled)
Comment #79
yoroy CreditAttribution: yoroy commentedCleaning out the "needs usability review" tag. The usability review is in #75
Comment #80
phillamb168 CreditAttribution: phillamb168 as a volunteer commentedReviewed all comments on the issue. After discussing with yoroy, consensus is that this issue is of ‘normal’ priority and should no longer be classified as ‘major’ because the bug is related to a misunderstanding of the way in which contact form has been changed from Drupal 7 to 8.
In Drupal 7, the contact form was really only ‘one form’ to which multiple categories could be assigned. Each category would have different settings pertaining to mail recipients, and different receipt email settings. However, the form itself was always displayed on the same page and additional fields were not available to it.
In Drupal 8, contact forms have moved towards a more webform/bundle based form system, where each new category is a fieldable bundle of the contact form entity type.(Or something like that.) Because of these changes, there is no longer a ‘valid’ need for a dropdown-based contact form selector. This functionality could be added back in to the basic contact form by adding a simple dropdown field, for example.
The issue summary should be updated to reflect the fact that this change is trying to provide some of the older D7 contact form functionality in a way that would be familiar to users of the D7 contact functionality.
Comment #81
andypostUpdated IS
Comment #82
andypostComment #83
xjm(Saving proposed issue credit for discussion and triage participants at LA.)
Comment #84
andypostAfter #2513396: There is no link, anywhere, to a contact form once a user creates it.
Now contact forms are linked to their routes from UI
Comment #85
jgeryk CreditAttribution: jgeryk commentedrerolling
Comment #86
andypostpatch #73 is rerolled wrongly - block is not included
Comment #87
Sharique CreditAttribution: Sharique as a volunteer commentedUpdated patch #85 with block included.
Comment #88
andypostThanx for re-roll, now new block needs tests
Comment #89
otarza CreditAttribution: otarza for World Food Programme commentedHello,
I found that contact navigation block wasn't working:
Last patch #87 was posted 6 months ago so it needed a reroll.
I did a reroll for latest 8.0.x branch, fixed bugs described above and created another patch.
So this comment includes 3 files:
contact-regression-reroll-to-8.0.x-1997692-89.patch
contact-regression-1997692-89.patch
contact-regression-reroll-fix-contact-navigation-block-interdiff.diff
Comment #90
otarza CreditAttribution: otarza for World Food Programme commentedComment #93
otarza CreditAttribution: otarza for World Food Programme commentedMy previous patches failed because I had reroll and bug fixes in separate patches, now this is merged patch.
interdiff.txt
contains diff between my previous reroll and new patch.Comment #94
otarza CreditAttribution: otarza for World Food Programme commentedComment #95
andrewsuth CreditAttribution: andrewsuth at World Food Programme commentedI can confirm that this reroll and block fixes (#88) works as expected.
+1 RTBC
Comment #96
andypostThis still needs tests
not sure this needed
contact form!
entity_type.manager
not sure that needed
interdiff
useless change
should be
return AccessResult::allowedIfHasPermission('access site-wide contact form');
Comment #97
andypostStill needs tests for new block
Comment #99
otarza CreditAttribution: otarza for World Food Programme commentedHello, I made changes according to @andypost's suggestions. Here is updated patch.
Comment #100
otarza CreditAttribution: otarza for World Food Programme commentedComment #102
andypost@otarza please provide interdiff to follow your changes
Comment #104
etron770 CreditAttribution: etron770 commentedAny news on this bug? I am using Drupal core 8.2.4. I have the same BUG as OP
Comment #107
Manuel Ferreira CreditAttribution: Manuel Ferreira commentedI Regret that this issue is still pending on D8.4.4
Comment #109
plato1123 CreditAttribution: plato1123 commentedHave contact form categories been removed? I see in the docs it's claimed different categories can go to different email addresses but I can't find that feature anywhere.
https://www.drupal.org/docs/8/core/modules/contact/overview
Can someone tell me where this option is? Sorry if this isn't the right place to ask about this, having trouble finding docs on this feature.
Thank you!!!!
Comment #118
smustgrave CreditAttribution: smustgrave at Mobomo commentedComment #119
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedComment #120
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedRe-rolled #99 patch.
Comment #121
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedComment #122
smustgrave CreditAttribution: smustgrave at Mobomo commentedLooks like there is an issue with the patch.
Comment #123
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedRectified Custom Commands Failed errors.
Comment #124
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedRectified Custom Commands Failed errors.
Comment #125
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound for Valuebound commentedtest passed.
Comment #126
smustgrave CreditAttribution: smustgrave at Mobomo commentedStill no tests added
Comment #127
smustgrave CreditAttribution: smustgrave at Mobomo commentedSo ignored patches 120, 123, and 124.
There were no interdiffs after 120 so didn't know what was being changed.
The reroll should have been tested before uploading. Adding the block and visiting the page broke the page because ->link() is not a function.
So starting at #99
Created tests for the block form and verify it's rendering
Tried creating an interdiff from 99 but it was so long ago the interdiff command returns
1 out of 2 hunks FAILED -- saving rejects to file /var/folders/z8/ym9ls3bj01x19hd8xfvsyk_40000gn/T//interdiff-1.1trJ2r.rej
interdiff: Error applying patch1 to reconstructed file
Comment #130
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #131
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedFixed CS errors
Comment #132
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #133
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #134
smustgrave CreditAttribution: smustgrave at Mobomo commentedThink this is ready to go. I worked on the tests #127 but not the solution.
Question for the committer is if this will need a change record.
Comment #135
larowlanAt this point, I think this is a feature request, and the presence of 'Create' in the issue title reinforces that.
I think its a good addition, but there's a bit to go still before I think its ready
for a mapping the keys must be known, so should this instead be a sequence where the keys are the contact forms and the values are the weights?
The remaining tasks says 'UX review' has that occurred for these new strings?
i'm not sure why we check for the block module here when outside the if we already link to the block display page? Shouldn't both links be inside the if?
these changes feel out of scope.
shouldn't this just be user.permissions?
also I don't think the cache context is cache_context.user.roles, it should be just user.roles, so that indicates missing test coverage I think
Should this be 'there are no contact forms yet'?
Should the 'create one' text link somewhere for users with appropriate permissions?
nit ===
We don't seem to have tests for this
We can make use of named arguments here and avoid the first two arguments which are just the default
why not use something that doesn't trigger cspell instead of adding the ignore? `contact_forms` would probably do?
there's nothing here that is testing the quite involved logic around ordering of disabled items at the bottom etc that's present in the block form, so I think we should add tests, or simplify that logic
Thanks for working on this again, let's keep up the momentum!
Comment #136
larowlanAlso we're missing calculateDependencies and onDependencyRemoval here
i.e. the block should depend on the contact form, and should remove it from the selected options when the contact form is deleted
Comment #137
larowlanAnd yes to a change record to trumpet the new addition 📣