Problem/Motivation
Currently the default weight property could be changed only by editing contact form.
Proposed resolution
Once we decide at #1997692: Create contact form block about weight property (2b or not 2b)
1) Allow to set default category within List UI
2a) Allow sorting on the same list builder
2b) Do not implement sortnig
3) cover with tests
Remaining tasks
cover with tests 1)
depending on state #1997692: Create contact form block extend patch or rtbc
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Update the issue summary | Instructions | ||
Update the issue summary noting if allowed during the beta | Instructions | ||
Update the patch to incorporate feedback from reviews. See comment #61 (include an interdiff) | Instructions |
User interface changes
1) Default category selected at the list
2) (?) Categories are sorted
API changes
ContactFormListBuilder
should implement FormInteface
or extend DraggableListBuilder
class for weight property
Original report by @Dave Reid
For D8 I'd really like to accomplish:
1. Implementing a drag/drop sorting of contact categories. We have a weights field, but we don't currently utilize it or allow people to change it.
2. Change the 'default' contact category to a radio on admin/structure/contact instead of having to edit the category you want to be the default and then saving the category. We should just implement this as a variable 'contact_category_default'
Related issues
#1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController)
#1891690: Use EntityListController for vocabularies
#1803638: Improve default language change process (ui and help text)
Comment | File | Size | Author |
---|---|---|---|
#45 | 599770-contact-sorting-45.patch | 8.48 KB | andypost |
Comments
Comment #1
joachim CreditAttribution: joachim commented+1 to both!
> We should just implement this as a variable 'contact_category_default'
I don't think we need to change the data storage, just the UI.
Comment #2
andypostThere's another UX issue #1477704: Replace "Selected" dropdown with checkbox in category edit form
EDIT and about storage #1588422: Convert contact categories to configuration system
Comment #3
andypostInitial working patch. Probably we needs tests.
Comment #4
joachim CreditAttribution: joachim commentedLooking good!
Will need some extra help text to explain what dragging them accomplishes.
Also, is 'selected' the one that's selected by default in the contact form? That maybe needs explanation too.
Comment #5
andypostSuppose fix for contact_help could go in follow-up, I'm not do good in English to write help texts.
Let's get more reviews from usability team
Comment #6
Bojhan CreditAttribution: Bojhan commentedLooks good, I am not sure we want a "selected" column - maybe it should be labeled "Default".
Comment #7
andypostChanged Selected to Default we are already changing this option in #1477704: Replace "Selected" dropdown with checkbox in category edit form to checkbox with text 'Make this the default category.'
EDIT related #1588422: Convert contact categories to configuration system
Comment #8
Bojhan CreditAttribution: Bojhan commentedCan this get code review, Dave Reid maybe?
Comment #9
andypost#7: 599770-contact-sorting-7.patch queued for re-testing.
Comment #11
andypostre-roll
Comment #12
andypostHere's a new patch, now ordering is enabled only when there's more then one category on site.
Probably it's possible to move form building into list controller... but not for now.
Comment #13
sunI agree this UI badly needs to be cleaned up. Also, better issue title.
But, hm. I agree we should not attempt to do this in a generic/abstract way just yet, but I do think that the new form code should be added to the CategoryListController override class... (?)
To do so, we'll probably have to use
drupal_build_form()
instead ofdrupal_get_form()
, but that shouldn't be a big deal.Comment #14
andypostI found similar implementation in #1787942-41: Allow assigning layouts to pages but Gabor used drupal_get_form()
Comment #15
sunWe've actually started with a generic implementation in #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController) already.
That patch is still waiting for #80855: Add element #type table and merge tableselect/tabledrag into it to land, but it looks like the generic implementation will boil down to a few lines only. Therefore, it might make sense to do that first?
Comment #16
andypostShould wait for #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController)
Related to default UI which similar to #1803638: Improve default language change process (ui and help text)
Comment #16.0
andypostAdded screenshot
Comment #17
andypostI don't think we should postpone this patch for #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController)
This form is not a default case and could be refactored later for default table-drag
Comment #18
geerlingguy CreditAttribution: geerlingguy commentedFollowing, and noting that it seems that adding non-default categories doesn't actually show them in a selection on the contact form. I added a new category, 'info', but didn't make it default, and no selection showed up on the contact page at all. I deleted the 'website feedback' default category, then the contact page said "You need to add a category" (something like that). I set the new 'info' category as default, and the form showed up again without an error. I don't know if whatever small bug caused this behavior could be fixed with this patch...
Comment #19
andypostNew patch build on top of FormInterface now this form is alterable by name 'contact_category_list_form'
@geerlingguy selection of contact category was droped but please file new issue to fix message in contact_site_page() or provide a fallback to first category if no category marked as default.
Comment #20
tim.plunkettNo, please don't combine FormInterface into EntityFormControllers in these issues. That's what #1913618: Convert EntityFormControllerInterface to extend FormInterface is for.
Comment #21
tim.plunkettI got FormController and ListController confused :) Sorry
Comment #22
andypostadded route conversion here
Comment #24
andypostand remains... related #1938386: Convert contact_category_* pages to a new-style Controller
Comment #26
andypostfix yml
Comment #28
andypost#26: 599770-contact-sorting-26.patch queued for re-testing.
Comment #29
podarok#26 looks pretty nice for me
happy thankspushing )
Comment #30
alexpottIt appears that we've completely lost the ability to select contact categories on the site wide form is missing. This should be postponed on #1997692: Create contact form block as that patch mandates the ability to have the ability to not have a default contact category. This is important functionality if you want to make people choose one!
Comment #31
andypostI think this is a great improvement in UI so let's find a compromise here.
While we can bikeshed about tabs/splitbuttons for UI the administrative sreen really lacks ability to re-order items!!!
So let's leave default as is, because we always have state when default category deleted
Comment #32
andypostDo not allow personal category to be selected as default
Comment #34
andypostFix test
Comment #35
tstoecklerAFAIK #disabled only sets the disabled attribute but does not actually prevent submitting that value. I.e. I think we should conditionally set #value. I mean something like:
Otherwise looks great!
Comment #36
andypostBetter display nothing at all
Comment #37
tstoecklerHmm, how about instead of
'#disabled' => $entity->id() == 'personal'
we just do
'#access' => $entity->id() != 'personal'
??
Anyway, this is super minor and shouldn't hold anything up.
If I would be a little more comfortable with contact.module I would RTBC #36 but I'll leave that to someone else.
EDIT: Fixed inverted condition
Comment #38
andypost@tstoeckler thanx #37 much cleaner
Comment #39
tstoecklerCoolio, thanks @andypost!
Comment #40
dawehnerCan I have some injections?
Comment #41
Bojhan CreditAttribution: Bojhan commentedCan the default setting, be something that is set on the configuration of the category? It can still be reflected in the table. But using a radio in a table has proven to be quite hard to use in previous testing.
Comment #42
andypostAddressed #40, suppose better to have a factory and default both
Working on implementation of changing a default category via operalins link
@Bojhan we still have a checkbox at the bottom of contact category editing form.
Comment #43
andypostright patch
Comment #45
andypostThere should not be links for config entities
also this should be re-rolled after #1938390: Convert contact_site_page and contact_person_page to a new-style Controller
Comment #47
dawehnerThis is looking pretty great.
Let's not mix up Symonfy and Drupal here.
This is a really great improvement.
Comment #48
BerdirNo, that is not an improvement.
That code was added there for a reason. It verifies that the Manage fields link is displayed, that was broken before.
Comment #49
tim.plunkettdrupalGet only verifies the routes, and doesn't actually test clicking around the UI. clickLink is important here
Comment #50
andypostThe both needed - to verify that link exists and accessible
Also related #1997692: Create contact form block
Comment #51
andypostDraggable is in #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController) so this should be re-rolled on top of it
Comment #51.0
andypostAdded related issues
Comment #52
alansaviolobo CreditAttribution: alansaviolobo commented45: 599770-contact-sorting-45.patch queued for re-testing.
Comment #54
alansaviolobo CreditAttribution: alansaviolobo commentedre rolled the patch.
Comment #55
andypostThe re-roll is wrong, the reason of the patch to add
formInterface
to list builder!Comment #56
larowlan@andypost are you still interested in working on this?
Comment #57
andypostYes, and this depends on #1997692: Create contact form block
Comment #58
prajaankit CreditAttribution: prajaankit commentedComment #60
andypostUpdate IS to current state
#58 shows hat tests are make sense
Comment #61
andypostContactFormListBuilder
should implementFormInteface
Comment #62
piyuesh23 CreditAttribution: piyuesh23 commentedComment #63
Patrick Storey CreditAttribution: Patrick Storey at Hook 42 commentedI am removing the Needs Reroll tag as the patch in comment 58 applies to head.
The steps I took to check this were to
1.) Make a new directory
2.) in the terminal: $ git clone --branch 8.0.x http://git.drupal.org/project/drupal.git
3.) $ cd drupal
4.) $ curl -O https://www.drupal.org/files/issues/clean_up_the_contact-599770-55.patch
5.) $git apply --check clean_up_the_contact-599770-55.patch
And I received no response so this patch applies to head.
Comment #64
Patrick Storey CreditAttribution: Patrick Storey at Hook 42 commented@piyuesh23 I am unassigning this task from you as I saw you were assigned it in Feb 2015. If you would still like to work on this issue just make another comment. :-)
Comment #65
Patrick Storey CreditAttribution: Patrick Storey at Hook 42 commentedI updated the remaining tasks to incorporate @andypost feedback in comment #61, there are some tasks in the original issue summary that I'm not sure whether they are relevant any longer. So we will need to update the issue summary.
Also we need a beta evaluation for this task. I'm going to remove the Novice tag until that is done and we're sure this is for the current version of Drupal 8.
Comment #66
andypostLast working prototype is #45
it needs to be rebuild on top of
DraggableListBuilder
and address comments #47
Comment #67
andypostComment #68
joachim CreditAttribution: joachim commentedThe languages admin page also has a weights + set default form, so that could do to be abstracted as a new builder class extending DraggableListBuilder.
Comment #73
drupgirl CreditAttribution: drupgirl commentedHi, not sure what patches I should be applying to get the the result of having the category dropdown in the contact form. Does anyone have this currently working on 8.4.3 have some quick pointers?
Comment #74
BerdirThere is no concept of a contact form category in Drupal 8, this just has a confusing issue title, should say default *form*.
The contact_storage module provides a field type that allows to send a contact form to different recipients based on a select. And webform might provide something similar.