Problem/Motivation
that suggests the user put the site into Maintenance mode. In working on a different issue I noticed this and figured it could be a quick ui improvement.
Steps to reproduce
Proposed resolution
Only display the instruction 'Put your site into maintenance mode' when the site is not in maintenance mode.
Remaining tasks
#48. Add a check mark when in maintenance mode is on
Add a new 'after' screenshot
User interface changes
Before with maintenance mode on. Also, after with maintenance mode off.
After with maintenance mode on
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#39 | 2099099-39.patch | 1.52 KB | quietone |
#38 | After.png | 72.29 KB | quietone |
#38 | Before.png | 76.81 KB | quietone |
#38 | 2099099-38.patch | 1.74 KB | quietone |
#18 | 2099099-18.patch | 1.35 KB | rpayanm |
Issue fork drupal-2099099
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
darrenwh CreditAttribution: darrenwh commentedI think a better option would be to omit this as a step opposed to striking it out as it is not a required task.
Comment #2
yurtboy CreditAttribution: yurtboy commentedmakes sense. I could make the patch do that instead if it is worth it.
Comment #3
yurtboy CreditAttribution: yurtboy commenteduploaded new patch to remove that line if TRUE
Comment #4
yurtboy CreditAttribution: yurtboy commentednow patch is named according to http://drupalladder.org/lesson/72a836e6-b408-a7a4-1923-8788761b2608
Comment #5
rpayanmComment #7
rpayanmComment #9
darrenwh CreditAttribution: darrenwh at Investis Digital commentedRe-rolling this patch
Comment #10
darrenwh CreditAttribution: darrenwh at Investis Digital commentedComment #13
opdavies#9 works for me.
Comment #14
rpayanmLooks good.
Comment #15
darrenwh CreditAttribution: darrenwh at Investis Digital commentedDo this need to back ported to D7?
Comment #16
opdaviesIt would need to be fixed in 8.0.x or 8.1.x first and then changed to 7.x, although there is a "needs backport to D7" tag that you can add if it needs backporting.
Comment #18
rpayanmReroll
Comment #19
darrenwh CreditAttribution: darrenwh as a volunteer commentedThis looks good
Comment #20
alexpottThis is not eligible during beta according to https://www.drupal.org/core/beta-changes.
Comment #21
darrenwh CreditAttribution: darrenwh as a volunteer commentedThis is a pity really, we have put a degree of effort into continually re-rolling it, this was created as part of code sprints in 2013 in Drupal Con in Prague and is endemic of the reasoning behind the continued delay of D8, not so sure if this is typical, but I feel whats the point in putting in this effort if its going to be put on the back burner? :(
Comment #22
darrenwh CreditAttribution: darrenwh as a volunteer commentedChanging Title
Comment #38
quietone CreditAttribution: quietone at PreviousNext commentedThis seems reasonable so I updated the patch and added screenshots to the IS.
Comment #39
quietone CreditAttribution: quietone at PreviousNext commentedThis should be better
Comment #40
quietone CreditAttribution: quietone at PreviousNext commentedComment #42
tedbowI was going to say this needs test but was easier to add it.
Switch to system.module because it is not in update.module(why I found this issue)
Comment #44
smustgrave CreditAttribution: smustgrave at Mobomo commentedEasy to test.
Applied MR 4524
Put site into maintenance mode
Verified line wasn't in update.php
Took out out of maintenance mdoe
Verified line was there
Comment #45
quietone CreditAttribution: quietone at PreviousNext commentedI'm triaging RTBC issues.
I looked at this late last night and after reading the IS and comments I don't see anything left to do. And thanks to @tedbow for adding a test. I ran the test locally without the fix and does indeed fail.
Leaving at RTBC.
Comment #46
lauriiiI'm not sure I agree we should be omitting the text about maintenance mode from the list when maintenance mode is enabled. To me, this page looks like a static list that describes the recommended steps for updating Drupal. I would not expect it to change depending on what steps I've taken before.
Comment #47
quietone CreditAttribution: quietone at PreviousNext commentedI'm triaging RTBC issues.
Reading the previous comment made me look at the IS again. The first image is now out of date and I am going to remove it. The screenshots to look at are in the "User interface changes" section.
Comment #48
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedRegarding #20, would not it be better from the usability point of view to have that point checked, or highlighted some other way to be clear, that the point is done? I also understand this text as a static guide, so I would not expect some points missing from it.
Comment #49
quietone CreditAttribution: quietone at PreviousNext commentedFor me, #46 and #48 imply that this is a won't fix.
I will ask in #ux for guidance.
Comment #50
smustgrave CreditAttribution: smustgrave at Mobomo commentedSo should this be closed?
Comment #51
quietone CreditAttribution: quietone at PreviousNext commentedLets get an opinion for a usability review to resolve this.
Comment #52
rkollerUsability review
We discussed this issue at #3386130: Drupal Usability Meeting 2023-09-15. The direct link to the recording of the meeting is: https://youtu.be/2bqTj97QiJ8?si=6-TmlJHwXvMVVtWJ&t=1260
For the record, the attendees at the usability meeting were @AaronMcHale, @benjifisher, @rkoller, @shaal, and @worldlinemine.
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
At first apologies for the belated comment. @quietone brought up the issue on the #ux channel last Friday. That issue looked quite familiar, so I did some digging in past issues before the meeting and it turned out we've already discussed it in September last year, where @quietone brought up the issue for the first time, but somehow we've failed to comment on the issue after the meeting. :( I've spent the weekend to watch the recording and re-familiarize with the issue. In the following a summary of the conclusions from back then.
There was a clear consensus among the attendees that simply hiding step 3 in case the requirement of an active maintenance mode is met is probably not the right thing to do here. Due to the fact you are having a numbered (ordered) list it might be a source for potential confusion if in one scenario (maintenance mode active) that list has three steps while in the other scenario (maintenance mode inactive) that very same list has four steps instead. That might be potentially problematic in the context of for example support questions referencing step 3 or 4.
The consensus what approach to take was in line with the suggestion made in #48, instead of hiding step 3 add a simple green checkmark or text illustrating that the requirement is met. On the other hand simply closing the issue as suggested in #50 would be problematic as well, without an indication like the suggested checkmark and having step 3 still in leaves the uncertainty for people with a small working memory if the maintenance mode was really already set or not.
We've also discussed adopting the pattern used in the
Verify requirements
step in the installe. But the problem as noted in #46 is, that the only requirement that is possible to be automatically tested in that list is step 3, the other steps are just recommended calls to action. Therefore doing anything more than just adding a checkmark would be probably not worth the effort.Aside that it might be probably a good idea to rework the micro copy of the
Drupal database update
page in general in a follow up issue. For example it isn't clear at all what doesUpdate your files (as described in the handbook page linked above).
in step 4 actually refer to?The upgrading handbook link it points to doesn't contain any clues in regards of "Update your files". If you search for
update file
orupdate your files
you have no match - the linked https://www.drupal.org/updating-and-upgrading-drupal-core was last updated on the 5th of December 2017. It might make sense to review if that page might need an update as well. At least as a resource in the context of the provided link on theDrupal database update
it is not that helpful at the moment.On a side note, directional instructions on the
Drupal database update
page like "steps above" pointing to the handbook page should be avoided. It is not advised to utilize any directional instructions/language that requires the reader to see the layout or design of the page (for example https://styleguide.mailchimp.com/writing-for-accessibility/).But all of those points are out of the scope for this issue and should better be tackled in follow-up issue(s).
I'll remove the
Needs usability review
tag and change the status back toNeeds work
Comment #53
quietone CreditAttribution: quietone at PreviousNext commented@rkoller, thanks for responding so quickly!
I have updated the remaining tasks to indicate a check mark is to be added. And I made the followup for the item in the last paragraph, #3414666: Improve text on database update page
Comment #54
quietone CreditAttribution: quietone at PreviousNext commented