Problem/Motivation
Config handler form in Views UI are currently missing their 'select all' behaviour, the JS in views-admin.js that is meant to be doing this does not work.
Proposed resolution
Fix the selectors and the JS. Form Ids Are now running through html ID I think, so e.g. what before was always 'views-ui-config-item-form', can now easily be 'views-ui-config-item-form--4' etc..
So let's add a 'views-ui-config-item-form' class to the form too, and use that. That's better than relying on any form ID or selector like 'form[id^=.views-ui-config-item-form]' or something.
It's also better to use the change event instead of click, so the checked property is always correct, and we should use prop() instead of attr().
With these fixes all is good again.
Remaining tasks
Review, manually test
User interface changes
Fix select all, it will appear again
API changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#28 | Screen Shot 2020-10-01 at 9.44.52 AM.png | 109.63 KB | rajneeshb |
#16 | config_handler_select-2452321-16.patch | 3.71 KB | Mac_Weber |
#10 | Content__Content____Site-Install.png | 35.15 KB | damiankloip |
#10 | interdiff-2452321-10.patch | 806 bytes | damiankloip |
#10 | 2452321-10.patch | 3.17 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedComment #2
dawehnerManually tested before and after and its great that the select all checkbox is there again.
The issue summary feels like being enriched with gems
Comment #3
alexpottFor some reason this select all thing kind a weirds me out - what happens if you have a bundle named "select" - i know that would be an odd name but anything is possible.
Comment #4
damiankloip CreditAttribution: damiankloip commentedHmm, ok. It feels like that should be discussed in another issue though, as this is just fixing a bug?
Comment #5
Bojhan CreditAttribution: Bojhan commented@damiankloip This could be solved by simply putting an small indent on the content types.
Comment #6
damiankloip CreditAttribution: damiankloip commentedThanks Bohjan, that seems pretty reasonable.
Comment #7
damiankloip CreditAttribution: damiankloip commentedOk, here is an updated patch. I realised there was broken CSS we had before creating a border around these options too. So how about this?
Comment #8
Bojhan CreditAttribution: Bojhan commentedAwesome, lose the borders :P
Comment #9
alexpott@Bojhan so it should look like this...
Or should or should the inner line remain and the indentation?
Comment #10
damiankloip CreditAttribution: damiankloip commentedok, all borders removed now.
Current status:
Comment #12
damiankloip CreditAttribution: damiankloip commentedSorry. Poorly named interdiff file.
Comment #13
Bojhan CreditAttribution: Bojhan commented(Y)
Comment #14
NickDickinsonWildeWorks as far as I can see perfectly and is in my list of auto apply before starting a new D8 project because not having a select all checkbox is very annoying.
Comment #15
alexpottNeeds a reroll...
Comment #16
Mac_Weber CreditAttribution: Mac_Weber as a volunteer commentedrerolled
Comment #28
rajneeshb CreditAttribution: rajneeshb as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedIt's working fine on 8.9.x.