UX: Use modal dialogs for confirmation forms. [#617730]

Comment	File	Size	Author
#3	617730.patch	2.05 KB	robloach
#1	617730-1.altered-confirm-before.png	23.24 KB	dww
#1	617730-1.altered-confirm-after.png	58.59 KB	dww
	deleteconfirmdialog.patch	1.97 KB	robloach

Comment #1

we/he/they

commented 29 October 2009 at 07:42

Status:

Active

» Needs work

Status	File	Size
new	617730-1.altered-confirm-after.png	58.59 KB
new	617730-1.altered-confirm-before.png	23.24 KB

This is pretty slick. ;) In the default case, it works well. However, when I altered my node_delete_confirm form from a contrib module, it only sort of worked. The fieldset was rendered (although with a pretty weird style -- all caps, etc), but my checkboxes just showed up as text, not as actual form elements. :(

Here's the altered confirm form before the patch:

And here it is as the dialog with the patch:

Log in or register to post comments

Comment #2

Bojhan commented 29 October 2009 at 09:59

Noooooo..... its bad we had confirmation screens,we are now going to put them in annoying popups as well? Bah

Log in or register to post comments

Comment #3

robloach

he/him

commented 29 October 2009 at 16:40

Status	File	Size
new	617730.patch	2.05 KB

The fieldset styling that dww got was inherited from Seven's CSS. Having confirmation screens is annoying, but its quite necessary when deleting content. This patch tries to make it less annoying by sticking it in a modal dialog box instead of needing a full page switch.

This patch fixes the title.

Log in or register to post comments

Comment #4

yched commented 29 October 2009 at 18:31

Gosh, I've been waiting for *years* for confirmation forms to use popups.
The most annoying part about confirm forms is the additional page load time. It seems that the D7 page rendering changes (an the fact that blocks are not computed if not displayed), should make the popup content, even retrieved through AHAH, much faster than a full page load.

Log in or register to post comments

Comment #5

robloach

he/him

commented 29 October 2009 at 21:00

Technically, this patch is a whole page load as well, but it uses the jQuery Load function to retrieve and show only the form.

    // When the dialog box displays, load the delete form and display the button.
    $form['#attached']['ui'][] = array(
      'action' => 'load',
      'selector' => '#dialog-confirm',
      'arguments' => array(base_path() . "node/{$node->nid}/delete form.confirmation", array(), 'loadComplete'),
      // ...
    );

The "action" states that we want to use jQuery.load, and the "arguments" say that we want to retrieve node/#/delete but only show "form.confirmation", which ends up just displaying the confirmation form in the dialog box. #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.) would've been really nice because then we could load forms remotely through Form API via JSON. Even though it is technically another page load, it is still saving the user a lot of time, as their browser doesn't have to re-render the whole page and load it's associated JavaScript/CSS. It just displays the form in the popup without the browser needing to change the pages. In the end, it saves the user some time, and improves usability.

Log in or register to post comments

Comment #6

Bojhan commented 29 October 2009 at 21:26

Sure, lets do it - just on principle hating dialog boxes for confirmations (*cough* Vista). Can anyone make a reviewable patch, all of this adds weight to getting dialogs in.

Log in or register to post comments

Comment #7

robloach

he/him

commented 29 October 2009 at 22:00

I think the only thing missing for this stuff is touching up #87994: Quit clobbering people's work when they click the filter tips link, and then getting that in.

Log in or register to post comments

Comment #8

seutje commented 9 December 2009 at 12:18

subscribe

just like to point out that node delete form is currently not being rendered in the admin theme, but the form you are displaying the popup on is

this could potentially mean that you're taking a form as themed by theme X and showing it in the context of theme Y

fortunately, in this case it doesn't rly matter, but I can think of use-cases where it does matter

Log in or register to post comments

Comment #9

scroogie commented 13 January 2010 at 10:28

subscribing

Log in or register to post comments

Comment #10

robloach

he/him

commented 13 January 2010 at 23:33

The #attached ui API patch doesn't have the function callbacks anymore.

Log in or register to post comments

Comment #12

Bojhan commented 24 April 2010 at 08:33

Version:

7.x-dev

» 8.x-dev

Log in or register to post comments

Comment #13

dealancer commented 14 September 2011 at 19:48

subscribing

Log in or register to post comments

Comment #14

Everett Zufelt commented 14 September 2011 at 20:00

Issue tags:

+Needs accessibility review

Log in or register to post comments

Comment #15

cweagans

He/Him

English

Boise, ID, USA

commented 14 September 2011 at 20:09

Title:

Usability: Popup node delete confirmation form

» UX: Use modal dialogs for confirmation forms.

We should do this for all of our confirmation forms. Retitling.

Log in or register to post comments

Comment #16

Everett Zufelt commented 14 September 2011 at 22:20

IMO we are going to need to complete #1175830: Update to jQuery UI 1.10.2 before tackling this issue. If we are going to use modals in Core we should abstract the functionality.

Log in or register to post comments

Comment #17

andypost

he/him

Russian

commented 21 September 2011 at 22:43

subscribe

Log in or register to post comments

Comment #18

Dharmendra.s commented 4 June 2012 at 16:21

i am using this patch but it's not working in drupal6 , i added this patch but nothing to happen ....so anyone can help me...

Log in or register to post comments

Comment #19

seutje commented 6 June 2012 at 11:45

The issue is currently marked for version 8.x, but the patch was attached when the version for the issue was still 7.x.
Additionally, the issue is marked "needs work", not "needs review".

So if it did work on 6.x, it would be purely coincidental. I would imagine applying the patch to fail.