Problem/Motivation
For binary options we should use a checkbox, not a "yes" "no" radio.
Proposed resolution
The checkbox can say "Exposed form in separate block"
The description could likely also be added to the bottom of the checkbox (not on top). Saying "Removes exposed form from view. Creates disabled block on blocks page ". Where "blocks page" is linked.
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#36 | view-exposed-screen-02.png | 1.97 KB | Himanshu5050 |
#36 | view-exposed-screen01.png | 2.4 KB | Himanshu5050 |
#36 | exposed-views-form-checkbox-2489406-36.patch | 1.95 KB | Himanshu5050 |
| |||
#34 | exposed-views-form-checkbox-2489406-33.patch | 1.45 KB | pguillard |
#27 | exposed-views-form-checkbox-2489406-27.patch | 1.45 KB | pguillard |
|
Comments
Comment #1
aburrows CreditAttribution: aburrows as a volunteer commentedI'm a mentor and working on this issue at DC LA
Comment #2
pguillard CreditAttribution: pguillard commentedOops, I started to work on it too. I let you make the patch, and maybe I'll review it!
Comment #3
pguillard CreditAttribution: pguillard commentedAll right, just because I made a patch, works ok, and passes test. Hum..will be for next time..
Comment #4
aburrows CreditAttribution: aburrows as a volunteer commentedSubmit your patch, and I'll test if it works will save me from doing it
Comment #5
aburrows CreditAttribution: aburrows as a volunteer commentedComment #6
aburrows CreditAttribution: aburrows as a volunteer commentedComment #7
aburrows CreditAttribution: aburrows as a volunteer commentedComment #8
Bojhan CreditAttribution: Bojhan as a volunteer commentedComment #9
lauriiiComment #10
aburrows CreditAttribution: aburrows as a volunteer commentedScreenshot attached again
Comment #11
aburrows CreditAttribution: aburrows as a volunteer commentedComment #12
pguillard CreditAttribution: pguillard commentedI reviewed your patch, it is working fine.
Added 2 more screenshots when checkbox checked/not checked.
Comment #13
lauriiiRTBC++
Comment #14
lauriiiComment #15
xjmThanks for the manual testing and the screenshots! I've embedded the main screenshot in the UI changes section of the summary for easier review.
So the title of the modal is "Put the exposed form in a block" and the checkbox is "Expose form in separate block". The "Put" is a little weird as a header anyway, and the checkbox text can be misread since it's missing some articles (I blame English for this).
Is there any reason not to make them say the same thing, i.e. (with fewer missing articles), "Expose the form in a separate block" for both? @Bojhan, others, thoughts? Setting NR for feedback on that suggestion.
As an aside, we should add beta evaluations for normal tasks, in general. In this case, since this currently only changes UI strings, it's unfrozen. Thanks!
Comment #16
aburrows CreditAttribution: aburrows as a volunteer commented@xjm I agree, when discussing this issue with several others at DC they seemed confused by the term Put, we can change to something like "Display exposed form as a block"
Comment #17
aburrows CreditAttribution: aburrows as a volunteer commentedUpdated patch so modal title says: "Display exposed form as a block" and the checkbox #title says "Expose form in separate block".
As per discussion with xjm.
Comment #18
Bojhan CreditAttribution: Bojhan as a volunteer commentedHmm, well we refer throughout the interface to a "exposed form" which is why I put it like that. We can diverge that is fine.
Comment #19
aburrows CreditAttribution: aburrows as a volunteer commentedComment #20
Zerdiox CreditAttribution: Zerdiox at iO commentedI've checked this at Drupalcon Barcelona 2015 against the latest beta version.
Patch still works and can be implemented.
I'll second adding the description under the checkbox that is proposed: Removes exposed form from view. Creates disabled block on blocks page
This behaviour is sometimes not expected by sitebuilders and is a good reminder. I've also added this as a new patch, should this not be necessary the patch from comment 17 works as is.
Comment #21
sam0971 CreditAttribution: sam0971 at iO commentedI tested the patch from #20 and it works. Checkbox is appearing in exposed filter settings.
Comment #24
aburrows CreditAttribution: aburrows as a volunteer commentedBoth patches work, RTBC++
Comment #25
xjmThanks for your work on this issue!
Why are we adding this description? I don't think we should. It seems out of scope for the issue and we have done a lot of work to remove unnecessary descriptions in the Views UI.
Since this issue is a string change, it has an rc deadline, so tagging so that it will get more visibility in case the patch is updated before then.
Comment #26
xjmAh, I think I understand why the description was added (re-reading the summary). We should only have one description on the form, not both. Illustration attached.
I also think the suggested description is a bit too confusing and terse, so maybe it would be better to change the form element only (so merely remove that hunk from the patch) and then change the description in a separate issue where we discuss the improved wording. Note that this change may also be added in 8.1.x if it's not done by RC.
Comment #27
pguillard CreditAttribution: pguillard commented@xjm : Applied the quick remove as of #25 because it is needed for RC deadline.
I allow myself to set it to RTBC, as it was before.
Comment #29
catchComment #30
catchThis is the sort of change we can make in a minor version, so if it misses the release candidate deadline it'd be fine for 8.1.x or later. Tagging 'minor version target'.
Also it doesn't seem 100% necessary to change the existing string in order to make this change.
Comment #31
pguillard CreditAttribution: pguillard commentedComment #32
dawehnerSo what I don't understand is why we cannot change strings when we also provide an update path for them at the same time? Wouldn't that solve the issues with existing translations?
Comment #34
pguillard CreditAttribution: pguillard commented@Catch : I didn't see your comments..
Comment #35
pguillard CreditAttribution: pguillard commentedComment #36
Himanshu5050 CreditAttribution: Himanshu5050 at Publicis Sapient for Publicis Sapient commentedReviewed patch from #34 and it is working fine.minor change applied i.e.
Exposed form should be displayed as a block (Screen 02) not in a block (Screen 01).
Screen 01:
Screen 02:
Comment #37
Himanshu5050 CreditAttribution: Himanshu5050 at Publicis Sapient for Publicis Sapient commentedComment #38
subhojit777Before applying patch:
After applying patch:
Patch looks good, hence RTBC :)
Comment #39
Bojhan CreditAttribution: Bojhan as a volunteer commentedComment #40
mikeker CreditAttribution: mikeker as a volunteer commentedDuplicate of #1969594: Make "expose form in block" UI easier to understand?
Comment #41
dawehnerIt is indeed. I don't care which one is committed, at all.
Comment #42
cilefen CreditAttribution: cilefen commentedSince the maintainer doesn't care, I prefer the checkbox. However, being newer, this is a duplicate of #1969594: Make "expose form in block" UI easier to understand. Can those who have been working on this post a patch over there?