Improve dropbutton

Tagging.

Setting priority to major as per http://drupal.org/node/1608878#comment-6550574

Added tags from that issue.

Each drop button gets at least 3 classes added via js and an element added (the toggle). If that is going to happen at 50 or more drop buttons' its going to have an impact on mobile performance.

I'd propose to move all classes to php, change the CSS to .js .class so it (still) only runs when js is enabled. Possibly also add the button already via php and hide it by default.

Since dropbutton got commited, this one is active I guess?

FYI: I created #1805488: Dropbutton is causing a horizontal scrollbar to appear

I dont see how this issue is actionable, unless a patch comes up I say you split of those issues into seperate ones.

Here is one important bug:

Dropbuttons are overlapping on a page if JS is off. (At least they did for me ...)

Let's start it off with this.

A few days ago I had a testbranch (which I stupidly deleted, gah), where I tried to fix some of my own complaints (see #3). Basically, I had some trouble in getting the classes in the right places using PHP instead of JS (though that might be fixed by moving some of the CSS around, nothing serious).

But the real thing I was having trouble with, mostly finding an elegant solution for it, is that if I want to add the button element in PHP (which is now added via JS, which in big tables almost certainly will be a performance hog for mobile -> DOM manipulation), I have to pretty much change the entire render array. Because at this point, the render type is 'links', which won't work if I want to add a button.

So it would need to be an 'item_list'?

Anyway, apart from the dom creation, we would benefit already by adding the CSS classes in PHP.
http://jsperf.com/foobarfoofroo

(test results shown are my chrome, which seems to produce low results, and my iphone 4 running ios 6)

The first half of #9 sounds like #1777326: Replace theme_links() with theme_item_list()
The second half you'll have to take up with all three JS maintainers who co-wrote this code...

Those numbers coincide with my small bench. Reducing the button JS rendering time by 40% by adding the classes in PHP is already a nice improvement, and much less invasive than adding the button via PHP.

(which can end up toreducing the rendering time by 40-80ms, depending on the browser maybe more, so IMHO significant)

We'll need to move the following code in node.admin.css to a more general place:

/**
 * Operations dropbuttons
 */
.dropbutton-wrapper {
  display: block;
  min-height: 2em;
  position: relative;
}
.dropbutton-widget {
  position: absolute;
  right: 0; /* LTR */
}
.dropbutton-wrapper,
.dropbutton-widget {
  max-width: 100%;
}
.dropbutton-multiple.open,
.dropbutton-multiple.open .dropbutton-widget {
  max-width: none;
}
.dropbutton-multiple.open {
  z-index: 100;
}

It was meant to give us a direction for styling dropbuttons inside of admin tables. I don't think we have a CSS file yet that spans all forms. Maybe we could add /core/misc/forms.css or move the CSS above into the individual themes?

At the moment, I'm leaning to moving the code into the Seven and Bartik themes. Let the dropbuttons be statically positioned in Stark.

#15 that css is now in dropbutton.base.css

I re-opened (and re-titled) #1805488-10: Dropbutton is causing a horizontal scrollbar to appear for the scrollbar problem.

@attiks, ok, I thought I was looking at the right patch, but I guess I wasn't. So I'll propose the change I described in #15 and see what folks think about it.

Only thing I notice is that after opening the operations I get an horizontal scrollbar, that isn't going away after closing the operations. Row height/Column width stay the same.

Removed some redundant jQuery wrapping: jQuery.fn.slice already returns a jQuery collection, no need to wrap it again.

When JavaScript is disabled, the widget is still absolutely positioned, this was causing them to overlap under certain conditions, making some links unreachable (with the pointer). (see attached screenshot)
I moved the absolute positioning to .dropbutton-multiple .dropbutton-widget selector, so it only gets applied when JavaScript is enabled and there are multiple items. This meant that I had to float the widget when there aren't multiple items (otherwise, it wouldn't be at the right side of the container).
I also removed one cursor: pointer as it was causing a pointer cursor when hovering the whitespace to the left of the widget.

The scrollbar issue (#1805488: Dropbutton is causing a horizontal scrollbar to appear) seems to be related to the .element-invisible class on the toggle button's inner span. since it has a (forced) 1px height, and width auto, it still takes up space, effectively sticking out of its container.

IMO, this is a bug with .element-invisible, but for the sake of moving this issue along, I slapped a overflow: hidden; on .dropbutton-arrow.
Feel free to discard this patch and move the .element-invisible thing to its own issue.

These changes work well for views, it let's us remove about 20 lines of custom CSS to do pretty much the same thing. I'm attaching the Views patch here for my own sanity.

My additions follow the suggestion of #3, but for different reasons. The custom styling makes no sense with JS off, so I added the class.

Still remaining is the fact that on wide screens, it looks ridiculous to have it all the way to the right in a table. Views is manually overriding that still.

Adding my first patch, please let me know if I've missed something.

This patch adjust the dropbuttons in /views_ui/css/views-admin.theme.css page. I've floated the button to the right if js is present and left the absolute position when js enabled. This prevents the overlapping and is accessible when js is disabled.

#24, thanks for the patch, but that's unrelated to the core issue.
If you'd like, open an issue in the Views queue. Thanks!

Back to needs review for #23.

I noticed this created a pretty big bug floating everything to the right, so I would love to see this in.

It seems that right-aligned buttons in table cells are not finding much favor. Since I'm in the minority on preferring it, I'll concede the issue and go with consensus. I made a few tweaks to this end (see the interdiff.txt). Nothing major I hope. Other than these changes, the rest of the patch looks good.

Left aligned dropbuttonsbuttons. Well, it's really no alignment. They line up with positioning origin.

Right aligned dropbuttons in RTL.

No-JS screenshot

Hiding overflowed dropbutton text.

so this doesn't happen

I assumed the right alignment was a requirement, but I like it a lot more when it's left aligned.

I still noticed 1 issue though, on very small window sizes, once triggered, the right side of the dropdown will be out of the viewport, causing a scrollbar to appear. But I wouldn't say it's a blocking issue, as it's only the last bit of the operation that might be a little hard to read for some people.

Happy to commit this patch as a starting point once it's RTBC. It's unclear to me if this resolves the "major" part of this task.

I marked #1781406: Use the new Core dropbutton as a duplicate of this issue now that Views is in Core (yay!). Any changes made here should also be reflected in the Views CSS.

Added focusin and focusout handlers for keyboard navigation, they just route to hoverIn and hoverOut, but I kept them in their own functions so it's still possible for other scripts to swap them out and make the logic differ from hoverIn/Out.

was that the "major" part webchick was referring to or is there anything else?

I think we need @sun to chime in here, but to #33:

was that the "major" part webchick was referring to or is there anything else?

We have:

Issues that need to be addressed.

in the issue summary.

Could you give an update of what is fixed of this within #33?

1. Make a11y compliant: The current dropbutton has not passed accessibility test and needs to be a11y compliant (#162)
2. Remove the z-index hack to fix sticky headers: Instead get #1780852: Sticky tableheaders bleed-through to work. (#105)
3. Fix dropbutton keyboard behavior: Currently the dropbutton stays open when navigating away from it. (#109)
4. Fix dropbutton to work in all UI scenarios: Needs to pass visual tests (#118)
5. Change markup to be compatible with JQuery UI / Twitter Bootstrap: Maybe use JQuery UI directly? (#126, #144)
6. Maybe use a native select element? (#110)

1: That link doesn't rly tell me anything about what needs to change
2: Not addressed (but #1780852: Sticky tableheaders bleed-through makes me think this issue doesn't try to solve that, but rather marks it for removal once it does get fixed)
3: Addressed (I think)
4: Adressed (I think)
5: I intentionally didn't touch the markup as there doesn't seem to be any consensus on this, everybody wants their own markup... personally, I find it mighty awkward that the trigger for opening the dropbutton is the second <li> and the rest gets this secondary-action class, or whatever...
6: see 5

Responding to #35:

1. Follow-ups at #1824634: [meta] Dropbutton accessibility issues and #1826368: Place dropbutton operations after their heading
2. #1780852: Sticky tableheaders bleed-through
3. Is addressed
4. Is addressed
5. Feature request
6. Feature request (that I'd like to won't fix)

So, I think we're done here.

Looks good. Committed to 8.x. Thanks!

Well, we never actually tested this in the actual Views UI.

Before:

After:

Sorry, correct patch.

Yes, this patch wasn't ready yet. Like, at all.

The follow-up fix in #39 is required now. The issue should move back to active afterwards.

Tim's patch fixes this for me.

Ok, committed and pushed that. Thanks!

I don't particularly see why we need to keep this issue open now, though. Seems like #36 showed that the bugs have follow-ups, and we can create some for those features that don't. I'm not sure how we're going to solve the remainder of the items in #36 any better at 43 comments in.

oh god, looks like FF16 interprets the unprefixed version differently from how FF before 16 interpreted the prefixed version
basically, spec says "0deg means bottom to top" and this is how the unprefixed version is implemented in FF16 (and up), but the original proposal by apple said "0deg means left to right"

so I guess our unprefixed declaration should use 180deg, while the prefixed versions should use -90deg or we use the keywords (but only for the unprefixed version), because right now, our implementation conflicts with the spec, so when everyone drops the prefixed versions, it'll be left-to-right on all browsers...

attached patch changes unprefixed version to 180deg to match the spec

reference: http://stackoverflow.com/questions/12868704/why-did-firefox-16-change-th...

Back to NR then (or to followup-issue)

bot be tweakin!

#44: dropbutton-1799498-45.patch queued for re-testing.

Committed and pushed to 8.x. Thanks!

Related: #1824634: [meta] Dropbutton accessibility issues