UI bug: Put log message in collapsible fieldset if revisions not enabled [#101319]

Issue http://drupal.org/node/87474 restored the ability to add log messages to nodes. That was a very good thing!

However, the side-effect of that is that now this field is visible all the time for users with "administer nodes" privileges. While it's generally assumed that people with this permission will be technically proficient, this is not always the case; for example, forum moderators and such. They then start wondering what this field for, why it exists, what they're supposed to type in it, how to see what they typed, etc.

This therefore represents a bug in the UI, in that there's this field that's in most cases purely optional that's instead sitting there looking like a required field.

Proposed fix:

- Place log field in collapsible fieldset
- Make fieldset expanded by default if $node->revision is true.
- Otherwise, make it collapsed by default.

The downside is that enabling revisions one-time on a node involves four clicks: once to expand "Publishing Options", once to click "Create new revision", once to expand "Log message", and once to click in the box and begin to fill it in.

Generally though, I think the use case for revisions is to always leave them on or off for all nodes of a given type.

Comment	File	Size	Author
#20	node_revision_prepare_1.patch	1.66 KB	pwolanin
#20
#14	node_revision_access_1.patch	1.97 KB	pwolanin
#14
#12	101319-collapsible-log-2_1.patch	609 bytes	forngren
#12
#9	101319-collapsible-log-2_0.patch	2.08 KB	forngren
#9
#7	101319-collapsible-log-2.patch	2.13 KB	forngren
#7
#6	101319-collapsible-log.patch	1.21 KB	forngren
#6
#3	logmessage.patch	1.21 KB	forngren
#3
#1	patch_50_0.txt	1.24 KB	webchick

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 4 December 2006 at 20:29

Status:

Active

» Needs work

File	Size
patch_50_0.txt	1.24 KB

This was webernet's latest patch on this issue. Still needs some work.

Comment #2

forngren CreditAttribution: forngren commented 25 March 2007 at 15:21

Version:

5.x-dev

» 6.x-dev

Keeping up with 6.x-dev.

Comment #3

forngren CreditAttribution: forngren commented 25 March 2007 at 15:22

Status:

Needs work

» Needs review

File	Size
logmessage.patch	1.21 KB

Forgot patch and status.

Comment #4

BioALIEN CreditAttribution: BioALIEN commented 26 March 2007 at 17:10

This is perfect, subscribing.

I've always wondered why the log textarea wasn't shifted inside a fieldset. It always looked out of place so +1.

Comment #5

forngren CreditAttribution: forngren commented 1 June 2007 at 17:36

Assigned:

Unassigned

» forngren

Updated patch

Comment #6

forngren CreditAttribution: forngren commented 1 June 2007 at 17:37

File	Size
101319-collapsible-log.patch	1.21 KB

And the patch...

Comment #7

forngren CreditAttribution: forngren commented 1 June 2007 at 17:49

File	Size
101319-collapsible-log-2.patch	2.13 KB

And another flavor.

Comment #8

Dries CreditAttribution: Dries commented 2 June 2007 at 11:12

This:

+    if (!$node->revision) {
+      // Collapsed by default when "Create new revision" is unchecked
+      $form['revision_information']['#collapsed'] = TRUE;
+    }

looks like a complicate way to write:

    $form['revision_information']['#collapsed'] = !$node->revision;

An important usability improvement though!

Comment #9

forngren CreditAttribution: forngren commented 2 June 2007 at 11:30

File	Size
101319-collapsible-log-2_0.patch	2.08 KB

Updated according to Dries.

Comment #10

Dries CreditAttribution: Dries commented 4 June 2007 at 07:46

Status:

Needs review

» Fixed

Committed to CVS HEAD. Thanks.

Comment #11

pwolanin CreditAttribution: pwolanin commented 4 June 2007 at 16:19

possible problem with the patch, this relocated checkbox:

    $form['revision_information']['revision'] = array(
      '#type' => 'checkbox',
      '#title' => t('Create new revision'),
      '#default_value' => $node->revision,
    );

no longer has an #access check, so normal users can decide whether or not to make a revision. Is that desired? to be consistent with the former method, it should have:

 '#access' => user_access('administer nodes'),

Comment #12

forngren CreditAttribution: forngren commented 4 June 2007 at 16:32

File	Size
101319-collapsible-log-2_1.patch	609 bytes

Good catch.

Comment #13

forngren CreditAttribution: forngren commented 4 June 2007 at 16:33

Status:

Fixed

» Needs review

Comment #14

pwolanin CreditAttribution: pwolanin commented 4 June 2007 at 16:46

File	Size
node_revision_access_1.patch	1.97 KB

Also, this:

  // Always use the default revision setting.
  $node->revision = in_array('revision', $node_options);

and associated code should move to node_object_prepare() , so that hook_nodapi can use $op='prepare' to change $node->revision, etc.

patch attached to clean up these 2 aspects of revisions on the node form

Comment #15

Dries CreditAttribution: Dries commented 4 June 2007 at 16:47

Status:

Needs review

» Fixed

Committed. Thanks.

Comment #16

pwolanin CreditAttribution: pwolanin commented 4 June 2007 at 16:50

@Dries -do you want the other part of #14 as a separate issue, or did you not see it?

Comment #17

BioALIEN CreditAttribution: BioALIEN commented 5 June 2007 at 13:47

Status:

Fixed

» Reviewed & tested by the community

pwolanin, I think Dries missed your post and patch in #14 by a minute.

Comment #18

pwolanin CreditAttribution: pwolanin commented 5 June 2007 at 13:56

should a file a separate issue? I think the change makes sense (even for a backport).

Comment #19

Dries CreditAttribution: Dries commented 5 June 2007 at 20:58

Ah, I think we just crossed each other. A quick re-roll would be helpful.

Comment #20

pwolanin CreditAttribution: pwolanin commented 5 June 2007 at 23:13

File	Size
node_revision_prepare_1.patch	1.66 KB

re-rolled. Note again, the purpose of this change in the relative positioining of this piece of code is to let node/nodeapi modules set $node->revision, or potentially to change the node form based upon it.