Dialog module reorganization and changes. [#662764]

This patch is in response to an IRC conversation between Roger, RobLoach, and myself. I believe this takes us in the direction we discussed, as well as adding a close command and some documentation changes.

The main points of interest are:

Move anything not strickly dialog api related to another module (now called dialog_links)
Renamed example folder and dialog_example module to "dialog_examples" (To address the fact that it is showing off multiple sub-modules now.)
Added dialog_loading command to dialog.module
Added dialog_close command to dialog.module and dialog.js
Added and changed many code comments. Feel free to correct the language anywhere its needed.

Please review and let me know if anything needs changes for a commit.

This patch is against the D7 version, aka CVS HEAD.

Comment	File	Size	Author
#7	dialogrevamp.patch	20.35 KB	robloach
#6	reorganize.patch	18.2 KB	te-brian
#1	reorganize.patch	15.14 KB	te-brian
	reorganize.patch	15.13 KB	te-brian

Comments

Comment #1

te-brian commented 18 December 2009 at 00:51

Status	File	Size
new	reorganize.patch	15.14 KB

This fixes one documentation length issue and one typo in the dialog_command_close() function.

Comment #2

te-brian commented 18 December 2009 at 00:53

Title:

Dialog module reorganiztion and changes.

» Dialog module reorganization and changes.

Title typo

Comment #3

te-brian commented 18 December 2009 at 02:27

After some IRC talk, I'm going to make some revisions to this patch. So feel free to comment on it in general, but don't commit.

Comment #4

robloach

he/him

commented 18 December 2009 at 03:10

Status:

Needs review

» Needs work

te-brain and I talked about having:

dialog.module: The Dialog API stripped down to the bones. This contains the Dialog AJAX commands (dialog_display, dialog_close, dialog, dialog_xlazyloader, etc).
dialog_core.module: Adds Dialog stuff to Drupal core via dialog.user.inc (dialogs for user.module), dialog.node.inc (dialogs for node module), dialog.taxonomy.inc (dialogs for taxonomy terms and vocabularies), etc. Dialog Core would also come with administration to check which features become enabled.
dialog_examples.module: Some use cases of the Dialog's API, similar to how the Examples module is setup.

Comment #5

zroger commented 18 December 2009 at 15:08

I like the direction of this.

I haven't yet played with it, but one initial thought is that we should be using 'dismiss' instead of 'close' for consistency with the 6.x branch.

Comment #6

te-brian commented 18 December 2009 at 22:37

Status	File	Size
new	reorganize.patch	18.2 KB

Alright, this patch accounts for recent IRC decisions as well as Roger's request to change 'close' to 'dismiss'.

It also adds the dialog_core module. This provides the means to use a 'use-dialog' class on links, as well as adding some special handling for logging in, registering, and requesting a password via the dialog. Left TODO on this functionality are some '#ajax' quirks, such as an enter key press not triggering the submit button's ajax functionality.

More generally, there are also some ajax problems to deal with such as 403 and 404s not being handled well in the dialog. Ideally, php errors during an ajax requests would prompt an error message, rather than send the user to a json reponse. These are probably to be solved outside this module, however.

[Edit]: Applying the patch leaves behind an empty 'example' folder (the real one is now 'examples'). I'm not sure how to deal with this in a patch.

Comment #7

robloach

he/him

commented 21 December 2009 at 12:50

Priority:	Normal	» Critical
Status:	Needs work	» Needs review

Status	File	Size
new	dialogrevamp.patch	20.35 KB

Okay, I built off of what te-brian did, and moved in some of the concepts that Roger brought up in the DRUPAL-6--1 branch as well as rearranged a couple things....

Moved the user stuff from the example to modules/dialog_user/dialog_user.module.
Removed $_GET['dialog'] as that's just kind of ugly. Replaced with dialog_enabled.
We now have example/dialog_example.module, modules/dialog_user.module, etc to match the DRUPAL-6--1 setup.
Moved all the OMGEVERYTHINGSADIALOG stuff (drupal_core.module) to modules/dialog_contextual/dialog_contextual.module, which adds a dialog for all the contextual links.
Moved some of the stuff that should be part of dialog.module there (like the command_redirect).
Added the %dialog_js menu load handler. This allows you to either use dialog_enabled or the custom menu callbacks. Dialog User and Dialog Example uses the menu callbacks, while the contextual links use dialog_enabled.

Todo:

Something is missing with modules/dialog_user/dialog_user.module's dialog_user_login_callback. You guys mind taking a look? The drupal_get_form() call is incorrect....
There are a lot of places in Dialog User that we could expose the dialog. This just exposes it on the user block.
Dialog Contextual Links is pretty minimalistic, and I'm sure it could improve, if you get any ideas, shoot them!
Dialog Comments