The list of actions has a bad fallback when JavaScript is turned off. The functionality is still working, but there are no graphical clue for the user that it's possible to trigger the actions. I think this is bad usability.
Remaining tasks
Check the latest patch doesn't affect dropbuttons in the following themes:
1. Seven
2. Bartik
3. Classy
A good selections of pages to test are:
1. /admin/content
2. /node/add/article
3. /admin/structure/views/view/content
Comment | File | Size | Author |
---|---|---|---|
#31 | Screen Shot 2015-02-20 at 12.04.39 pm.png | 41.29 KB | aliyakhan |
#31 | Screen Shot 2015-02-20 at 12.01.42 pm.png | 60.91 KB | aliyakhan |
#29 | seven-3.png | 213.69 KB | betovarg |
#29 | seven-2.png | 160.35 KB | betovarg |
#29 | seven-1.png | 158.82 KB | betovarg |
Comments
Comment #1
LewisNymanAh I get it, these should look like buttons when JS is disabled right? I think we just need to add .js in front of some of the CSS?
Comment #2
juho.lehmonen CreditAttribution: juho.lehmonen commentedHere's a patch for a nicer no-js dropbutton fallback. I added .js before the background and border overrides, so those don't happen when javascript is disabled.
I also added some alignment and margin styles to align the buttons better without javascript. Any opinions on this? I'm pretty sure there's no risk of this bleeding through, but I suppose there might be a cleaner way of doing this.
Comment #3
juho.lehmonen CreditAttribution: juho.lehmonen commentedDisregard the previous patch, I forgot to reset the default ul styles for the fallback (padding, margin and list-style-type). This one should work.
Comment #4
Outi CreditAttribution: Outi commentedI tested and it looks good for the publishing buttons (the screenshot A).
I just checked how the buttons would be on a view editing page (admin/structure/views/view/content), and there are some problems with the patch (screenshot B) and without the patch (screenshot C), but I don't know if this should be added to this issue or not, because the patch #3 clearly fixes the original problem.
Comment #5
juho.lehmonen CreditAttribution: juho.lehmonen commentedIt seems to be a bug with disabled javascript in Views UI. The patch didn't really make the situation worse, it's just broken in a different way now.
Is there already an issue about no-js usability in Views UI? I could take a look at fixing this if no one else is currently working on it.
Comment #6
Outi CreditAttribution: Outi commentedNo, it didn't make it worse, the Views UI without JS just doesn't seem to be fixed. I think it should be fixed in another issue and consider this issue fixed with the patch #3.
Comment #7
LewisNymanAgreed! Views UI is too complicated, we should manage it on it's own.
Just a few minor code review points:
This requires a space before the value.
Instead of using 'li' we use the '.dropbutton-action' class. It would be nice to keep it consistent.
Comment #8
Outi CreditAttribution: Outi commentedActually the '.dropbutton-action' class seems to be created by JS (when JS is deactivated the dropbutton 'li's don't have it). I don't know if this is normal; so should the '.dropbutton-action' class be there only when JS is activated (and in that case we must use 'li' here and not the class), or should the class be there in all cases ?
Comment #9
Outi CreditAttribution: Outi commentedOk, LewisNewman told me that we can keep the 'li' as the '.dropbutton-action' class is created by JavaScript, so this patch adds only a space before the value on the 'list-style-type: none;'.
Comment #10
Outi CreditAttribution: Outi commentedComment #11
LoMo CreditAttribution: LoMo commentedLook great on my local Mac browsers, but I tried on Android and it needs work on smaller screens where the buttons are not in a row. See screenshot.
(According to the themer sitting to my left, we should: Remove the left margin from all of these buttons and add a margin-bottom.
Comment #12
LoMo CreditAttribution: LoMo commentedComment #13
LoMo CreditAttribution: LoMo commentedComment #14
LoMo CreditAttribution: LoMo commentedThis patch should get things okay. It only affects layout and adds just the margin-bottom and sets margin-left to zero.
Comment #17
LewisNymanThis fixes the problem. I also tested to make sure that there were no regressions to Dropbuttons with JS.
Comment #18
droplet CreditAttribution: droplet commentedbuttons should be 100% on mobiles. Patch is coming.
Comment #19
droplet CreditAttribution: droplet commentedComment #20
jerrylow CreditAttribution: jerrylow commentedLooks good - screen shots for iPhone simulator, tested it for different iOS devices too.
Comment #21
LewisNymanComment #22
amitgoyal CreditAttribution: amitgoyal commentedRe-roll of #19.
Comment #23
LewisNymanWe lost the LTR comment on this line, also I guess this means the selector below it also needs to be moved in to the media query?
Comment #24
amitgoyal CreditAttribution: amitgoyal commentedFixes as per #23.
Comment #25
LewisNymanGreat, thanks. We just need to take some screenshots of the JS dropbuttons in all the themes to show that it's not broken by these changes.
Comment #26
NGenius CreditAttribution: NGenius commentedHello,
I am making screenshots.
Comment #27
LewisNymanComment #28
betovargComment #29
betovargI reviewed the behavior on chrome 40.0.2214.111 (64-bit) and firefox 35.0.1 on mac, on the needed screens. I tested on a local machine and simplytest.me (I verified on both, but screenshots are only for my local environment):
/admin/content
Seven
Classy
Bartik
/node/add/article
Seven
Classy
Bartik
/admin/structure/views/view/content
Seven
Classy
Bartik
Comment #31
aliyakhan CreditAttribution: aliyakhan commentedLooks all good in D8 8.0.x already. Attached couple of shots.
Comment #32
aliyakhan CreditAttribution: aliyakhan commentedComment #34
Wim LeersThis should also help #2412945: Determine which additional asset libraries should be in the critical path/loaded i/t header (core/drupal, core/dropbutton)! :)
Comment #35
alexpottCSS changes are not frozen in beta. Committed 71a987d and pushed to 8.0.x. Thanks!
Comment #37
idebr CreditAttribution: idebr commentedThis issue introduced a visual regression in the dropbutton for users with javascript enabled. I have opened #2430999: Visual regression in dropbutton to fix this.