OK and Cancel buttons on a different line than "upload"/"submit" buttons violates Drupal UI standards

The reason we put the titlebar back is there jquery dialog has a bug where you can lose your buttons. I'm not sure exactly what causes it, but their div becomes display:none which basically means you have to refresh the page, which means you lose your story :(

But I guess we should fix that too, so maybe re-introducing that bug will help us feel inspired to cleanup the button mess.

Cool, would love to see what you work up. The scary thing there is that then the iframe is ref'ing stuff outside of it. This means it won't work on its own (for instance if it is used in admin/content/media).

Also, it wouldn't work if it was embedded in a page, etc. I'm okay with forgoing some flexibility for now for usability though.

My general goal is:

The media browser plugins have no knowledge of
the iframe they are contained in
the iframe has no knowledge of
the dialog it is contained in
the dialog has no knowledge of
the popup code which calls it
the popup code has no knowledge of
call sites who launch the browser (like media as a field, wysiwyg, the link on the admin screen, etc).

We can bleed these, but especially the wall between the call sites and the popup code needs to be totally insulated IMO.

Best,
Jacob

There is no select button on the library AFAICT.

Also, needs a cancel button of some sort.

okay, so I get it, there is a bug here. IT doesn't work w/ wyswiyg.

And then it started working... and I don't know why.

#morewhiskey

I really, really like how this patch makes the browser look.

I don't like how the code does it though.

Adding the fake buttons is a clever way to try to be non-intrusive in the form, but it's a kinda funky architecture, isn't it? I'm not saying I have a magic bullet "clean" solution, but I feel like we should do some more work here... Perhaps a cleaner way to start (For now) is to just add the buttons to the forms themselves in PHP. It means they won't be re-usable and it's kinda ugly in its own way, but less so IMO.

-J

Could we add some screenshots? Please?

#18 is an unholy hack, but let's face it, so is much of media browser javascript. Maybe the whole thing can be refactored some day, but until then, #18 is an improvement to HEAD. I discussed this with James, and each decision in the patch has a logical reason for being there, and the end result is to bring the media browser into closer compliance with Drupal UI standards.

Jacob #13:

Adding the fake buttons is a clever way to try to be non-intrusive in the form, but it's a kinda funky architecture, isn't it? I'm not saying I have a magic bullet "clean" solution, but I feel like we should do some more work here... Perhaps a cleaner way to start (For now) is to just add the buttons to the forms themselves in PHP. It means they won't be re-usable and it's kinda ugly in its own way, but less so IMO.

James #14:

Injecting the buttons via PHP and attaching javascript actions doesn't seem to be much more clean to me.

I agree with James. The #18 patch has the correct separation of what's in PHP and what's in JS. The JS respects the presence of a real submit button (if any) added by PHP. It injects additional buttons, if needed, whose only purpose is to trigger JS code.

What's hacky in #18 is that the media module has JS code to add "ok" and "cancel" buttons that it hides, and also to add "submit" and "cancel" buttons whose only purpose is to route control to the hidden buttons. Ugly, but apparently necessary to get around cross-IFRAME scripting issues. I'm attaching a patch that has no code change relative to #18, but adds comments to explain the non-intuitive JS code.

@James: if you're happy with the comments, please commit.

This code has been well tested by people with sites on drupalgardens.com.

.

Thanks. By the way, the change to dialog('destroy') was in #18, so I can't take credit for that :)