Great module. I think the vertical tab for Scheduler on the content type edit form can be improved.

Before:

annotated screenshot of scheduler settings on the content type edit form

"Check this box" is documentation for how a checkbox works. It also makes all descriptions start the same, whereas it is better to lead with the specific trigger words for each option. If you remove those bits from the descriptions, you see that they hardly differ from the checkbox labels. I tried rephrasing the checkbox labels to include any extra info that was in the related description.

After:

Now, this reduces the total height of this form quite a bit, so maybe the fieldsets do not have to be collapsed anymore. This reduces clicks and immediately shows you what can be done here. I also removed unnecessary instances of the 'settings' word from the fieldsets and vertical tab titles.

scheduler-settings-vtab-after-1.jpg

CommentFileSizeAuthor
#6 1333238-6-checkthisbox.patch4.11 KByoroy
#3 1333238-3-checkthisbox.patch4.09 KByoroy
#1 checkthisbox.patch4.12 KByoroy
scheduler-settings-vtab-after-1.jpg69 KByoroy
scheduler-settings-vtab-before-expanded.jpg102.14 KByoroy

Comments

yoroy’s picture

yoroy commented
Status: Active » Needs review
Issue tags: +Usability, +ui-text
StatusFileSize
new checkthisbox.patch4.12 KB

A patch, even :)

jonathan1055’s picture

jonathan1055 commented

Hi Yoroy,
I like this - anything which declutters a page to improve useability without losing information is a good idea.

Just a few observations

  1. The wording of 'alter published time to match scheduled time' can be confusing to users. What it actually means is that the node 'created' time (ie the datetime often shown to visitors) is reset to match the scheduled publishing time. What is the best way to say that?
  2. I think the titles should be wrapped in t(). I know the original was not, and issue #1199908: Some missing translations already noted this. That patch has not been committed yet, and it would have to be re-rolled anyway if this is done first, so you may want to add the t() here, it's up to you.
  3. If the published fieldset collapsible=FALSE then you can delete the line collapsed=FALSE
  4. I am ok with having the fieldsets not collapsible, but there was talk of introducing new options which would make the form longer. If/when that happens we can revisit, but for now I think it is good improvement to have the fieldsets non-collapsible

It can help if you name your patches with _issuenumber_commentnumber_description, so that they float to the top of file lists and are ordered and do not get mixed up with the actual module files.

Nice work, thank you.
Jonathan

yoroy’s picture

yoroy commented
StatusFileSize
new 1333238-3-checkthisbox.patch4.09 KB

Thanks for a good review.

1. Yeah, that's the sentence I was least happy with. I changed it to "Set content creation time to scheduled time".
2. I'm a coding noob so I'd rather not expand the scope of this patch.
3. I was wondering about that yeah :)
4. By the time new options are introduced it would be good to consider adding some progressive disclosure, for example, hide the 'Required' checkbox by default and only show it if the 'Enable (un)scheduling box is checked. Maybe all other options can be shown dependant on that first 'enable' checkbox.

Git is difficult for me, so I cut corners here and there :) Thanks for the info, updated the patch.

jonathan1055’s picture

jonathan1055 commented

@yoroy #3

  1. I don't see that change in the second patch. But how about 'Alter content creation time to match the scheduled publish time'
  2. OK. That other issue can deal with it
  3. yes it has gone
  4. yes I agree

@Bojhan #4
I think we are already simplifying the form enough. I like the visual grouping of the two sets of related options. The border does not take up much screen space, so I say leave the fieldsets in.

yoroy’s picture

yoroy commented
StatusFileSize
new 1333238-6-checkthisbox.patch4.11 KB

Lets do 'Change' instead of 'Alter', but yes 'Change content creation time to match the scheduled publish time'.

(I discussed the fieldsets a bit with bojhan in IRC and we quickly ran into other layout (spacing) issues without them so he agreed to leave the fieldsets for now)

jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Reviewed & tested by the community

All fine with me, patch applies cleanly and looks good.

eric-alexander schaefer’s picture

eric-alexander schaefer commented
Status: Reviewed & tested by the community » Fixed

Thanks for the patch. I like the new, concise look of the the node configuration tab.

http://drupalcode.org/project/scheduler.git/commit/1797243

yoroy’s picture

yoroy commented

Ah, glad to see this made it in, thanks :)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

before/after headings