Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Create a new revision is is a decision fork for the user. Assuming that they should do this and therefore expanding it by default adds visual noise, increases cognitive load and presumes we know what they want to do.
If the user wants to create a new revision they will check the box and create one. Let's not nanny them.
Here's what it looks like now:
Here it is collapsed:
image lost
Much cleaner and simpler.
Comment | File | Size | Author |
---|---|---|---|
#38 | drupal-make-create-a-new-revision-collapsed-by-default-2064505-38.patch | 511 bytes | andriyun |
Comments
Comment #0.0
tkoleary CreditAttribution: tkoleary commentedadded image
Comment #0.1
tkoleary CreditAttribution: tkoleary commentedadded image
Comment #0.2
tkoleary CreditAttribution: tkoleary commentedresized
Comment #3
tkoleary CreditAttribution: tkoleary commentedComment #4
larowlanTagging
Comment #5
Wim LeersI don't see clear differences?
Comment #6
tkoleary CreditAttribution: tkoleary commented@Wim Leers
Page loads with "Create new revision" unchecked
Comment #7
Wim LeersNone of the screenshots show that. Also, this is a configurable default, not an enforced default!
You can configure it for each content type at
admin/structure/types/manage/article
→ Publishing options → Default options → Create new revision.Please also take into account the implications beyond visual noise of having this disabled by default: it will mean that by default, Drupal will not protect the user against accidental mistakes, by creating a new revision each time!
Comment #8
tkoleary CreditAttribution: tkoleary commentedYes it's a configurable default but it still has a default in the content type, which is enabled.
Sure, that's great, but we can accomplish both ends by:
Now we reduced cognitive load and steered the user away from bad practice at the same time.
Comment #9
Bojhan CreditAttribution: Bojhan commentedI think the correct fix is not to separate this out in another collapsable item, but only opening this by default when a revision message is required. By default its not in core, therefore it will stay collapsed. A patch for that should be simple, and resolve Kevin's concerns.
Comment #10
tkoleary CreditAttribution: tkoleary commentedNot really because then we would need yet another checkbox for the user who wants to add a message right?
Comment #11
Bojhan CreditAttribution: Bojhan commented@tkoleary No, essentially - unless the "message" part of the revision is required. It will not collapse by default. Which means that it will still make a revision, but its not open by default anymore unless the message part is required.
Comment #12
tkoleary CreditAttribution: tkoleary commented@Bojhan
Oh, excellent. In that case your solution in #9 is perfect
Comment #13
Wim LeersI don't understand #9.
The revision log message has never been required. There is no way to make it required. Plus, when it's optional, the user should be able to enter such a message anyway.
So how then will the user be able to enter a revision log message?
Or are you arguing that there should be a new setting besides the "Create new revision by default" setting, called something like "Require a log message when creating a new revision"?
Comment #14
Bojhan CreditAttribution: Bojhan commentedI am arguing for the latter, adding that checkbox. Since its optional, you can always expand it - I don't see any trouble there.
There is already something like this on d.o, when you change a docs page.
Comment #15
Wim Leers#14: how would you expand it if it is invisible?
d.o docs pages just "hide" it in a fieldset, that is open by default? How is that hidden by default? Surely we don't want another fieldset?
I'm extremely confused. Clear before vs. after mocks would be most helpful.
Comment #15.0
Wim Leersresized
Comment #16
andriyun CreditAttribution: andriyun commentedComment #17
andriyun CreditAttribution: andriyun commentedComment #18
tvn CreditAttribution: tvn commentedComment #19
shumer CreditAttribution: shumer commentedPatch for review
Comment #20
clebu CreditAttribution: clebu commented@shumer: There is a small typo: '#title' => t('Add log messge'),
Comment #21
peterg.griffin CreditAttribution: peterg.griffin commentedComment #22
peterg.griffin CreditAttribution: peterg.griffin commentedTypo still exists: '#title' => t('Add log messge'),
Comment #23
peterg.griffin CreditAttribution: peterg.griffin commentedComment #24
peterg.griffin CreditAttribution: peterg.griffin commentedComment #25
pandaPowder CreditAttribution: pandaPowder commentedI am at Drupalcon Austin and I'm reviewing this patch.
Comment #26
pandaPowder CreditAttribution: pandaPowder commentedI was going to reroll this patch because it is stale, and I got it rerolled but I'm not going to upload it because after further consideration, I've decided that I don't like the proposed change. I feel that the extra checkbox for "Add log message" Overcomplicates the decisions the user has to make. Whether the log message field is displayed to the user or not, it is not required and adding the extra toggle visibility box just gets in the way.
Comment #27
SGhosh CreditAttribution: SGhosh commentedTried out the patch, it does need a reroll. But it doesn't seem a necessary addition.
What it does is add another checkbox for the user to add a message, but on checking this checkbox it only makes the add message textarea show up. It doesn not even make the revision message required. This infact seems to be an unneccesary addition to what is already present.
Moving this to minor priority.
Comment #28
SGhosh CreditAttribution: SGhosh commentedBut rerolling the patch anyway, for others to try out. Maintainers please close issue if this will not be worked upon.
Comment #29
JacobSanfordTriggering testbot on patch in #28. Regardless of any decision to proceed, it would be nice to know if patch is clean.
Comment #32
amitgoyal CreditAttribution: amitgoyal commentedReroll of #28.
Comment #33
andriyun CreditAttribution: andriyun commentedFatal error: Call to undefined function Drupal\node\user_access() in /home/sa08b0b10dc8ddca/www/core/modules/node/src/NodeForm.php on line 157
user_access function deprecated.
You should be use $current_user->hasPermission('administer nodes') instead user_access()
See core changes user_access() got converted to a method on the user/accountInterface
Comment #34
omers CreditAttribution: omers commentedOK here is the patch with the reroll, the interdiff and looks like that :)
Comment #35
omers CreditAttribution: omers commentedComment #36
andypostThe summary is not clear what was proposed!
/admin/structure/block/block-content
uses nearly the same widget so it makes sense to unify themComment #37
andypost#34 is no way, also no reason to abuse a cleaned node form
Comment #38
andriyun CreditAttribution: andriyun commentedHey guys!
To make block "revision information" collapsable it's enough to remove overriden this block in theme seven.
I do not know why it has been done, but file /core/themes/seven/seven.theme contain next code:
To make this block collapsed by default need to correctly set "#open" key in node form
Comment #39
andriyun CreditAttribution: andriyun commentedComment #40
andypostThe patch goes wrong way imo too. Also it pointless to scroll down to actually change authoring
I'd prefer to get rid of that element "revision_information" in favour of current approach.
It makes sense to input log message only when creating revision (checkbox checked)
so maybe just move code from seven to default implementation at least?
Comment #41
kfritscheComment #42
tkoleary CreditAttribution: tkoleary commented@andypost
Yes it makes sense to unify.
And revision should not be a special snowflake but handles as the other collapsed panels are.
Comment #43
Bojhan CreditAttribution: Bojhan commented@tkoleary Isn't it a special snowflake because we wish to elevate that one?
Comment #44
vasi CreditAttribution: vasi at Evolving Web commentedWe're going to look at this at DrupalCon LA.
Comment #45
vasi CreditAttribution: vasi at Evolving Web commentedIt's unclear what needs to be done here, so we're removing the "Novice" tag.
Comment #46
Wim LeersComment #47
Wim LeersComment #48
tkoleary CreditAttribution: tkoleary at Acquia commentedComment #49
tkoleary CreditAttribution: tkoleary at Acquia commentedComment #50
tkoleary CreditAttribution: tkoleary at Acquia commentedComment #51
tkoleary CreditAttribution: tkoleary at Acquia commentedComment #52
andypostAny reason to have this checkbox when creating a node? this is a first revision anyway!
Also node edit form has revision collapsed by default but should have open when there's revisions already
IS should be updated after usability review
Comment #64
quietone CreditAttribution: quietone at PreviousNext commentedSeven has been removed from core. This was discussed in #frontend and markconroy said that this should move to the 'theme system' component. I am doing that now.