Posted by coltrane on March 7, 2009 at 10:51pm
16 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | node.module |
| Category: | task |
| Priority: | normal |
| Assigned: | attheshow |
| Status: | closed (fixed) |
| Issue tags: | accesssibility, needs accessibility review, Needs design review, UBUserTesting2009, Usability |
Issue Summary
Identified at UB Usability Testing: http://www.drupalusability.org/node/148
The pink text on an unpublished node does not accurately indicate to the user that the node is unpublished.
Comments
#1
This patch makes a change to node/node.css, node/node.tpl.php, and garland/node.tpl.php to display the word "UNPUBLISHED" behind the body content.
I admit, this is totally ripping off the Zen theme, but I think Zen includes many great solutions for problems.
#2
#3
A screenshot would help move this along. Sounds proper to me.
#4
Good idea. Thanks. Screenshot is attached.
#5
well done.
#6
Agreed, very nice.
#7
Tagging as "needs design review"... I'm not sure how they're going to feel having all of that extra stuff added to node.css, and they might have some other suggestions on implementation.
#8
Just checking...this would be translatable, yes?
#9
Yes, the unpublished is run through t().
#10
Hmm… I wonder where I've seen this before. lol! #207743: Add Zengine Block Edit Links and "Unpublished" background
Yeah, the “drupal is slightly embarrassed this node is unpublished” idea isn't intuitive.
#11
Yes much better of course. The only way to solve this unambiguously is with actual text.
Any reason to still keep the pink then?
#12
A background of diagonal lines would also get the job done. Think of the background patterns on checks.
#13
@modulist: I don't understand how diagonal lines would say that content is not published...
#14
Do we need a background at all if we are using text anyway was my question, or are you suggesting to add the background image to the text solution? There is no graphics-only solution without using the actual 'unpublished' text that will communicate "unpublished" unambiguously imo.
The diagonal raster is an elegant idea, I like it but not fool proof. This is for the theme-independent solution in node.css, I think we should focus on being as clear as possible. A theme could add the diagonal raster.
3 weeks since webchick asked for more design feedback, I mentioned it 2 times in the IRC design channel during that time. Let's do it.
#15
Eh sorry. Let's remove the background color first, it's meaningless.
#16
Removed the background color. Attaching updated patch.
#17
New screenshot, please?
#18
Sure thing. Attached.
#19
Works fine, one minor thing, do we have to show UNPUBLISHED on node preview too?
#20
Probally not.
#21
Added three lines to node.css
.node-preview div.unpublished {display: none;
}
works fine
#22
Oke, so I am going to mark this RTBC. It seems that this indicator is really good, and I see no reason not to go forward with this. As yoroy his last question was to remove the body background, that has been done - so I think all issues have been addressed.
#23
Committed to CVS HEAD. Thanks!
#24
Automatically closed -- issue fixed for 2 weeks with no activity.
#26
I don't see this as having been committed.
While I am for adding the textual message the implementation just seems like a quiet work-around the API freeze - this should be in a variable available to all themes, not hard coded using t() in a template file.
Also the styles. We already have way too many design styles coming from modules - themes should style this output, not the module.
Isn't this an accessibility issue?
#27
Tagging. Would be great if when things were committed to core that there was a link to the code & commit message. I think Redmine & other tools do this, but a Git Drupal solution would be very cool. Well, hopefully for D8..
That being said, has there been a review with tools like http://wave.webaim.org/toolbar
#28
mgifford: http://drupal.org/project/cvs/3060 or follow http://twitter.com/Druplicon :)
#29
Thanks yoroy! I was just dreaming about an integrated system where there were direct links to the repository when stuff got into core. I should really put this in a feature request though.....
#30
My eyes are pretty good. I had a hard time even seeing the pink background myself. Switching it to having the text Unpublished in the background is just so much better!
+1 for this patch for core themes. It's definitely the right approach.
This is an accessibility issue for sure. It's also a usability issue. If even with good eyes the pink doesn't stand out it's really not very useful.
It would be useful to see how a blind user deals with this.. It is adding a whole new level of information that they wouldn't have ever been able to know about.
VoiceOver read it correctly. I think it's good.
#31
Looking at the related issue #867830: "unpublished" status of rendered entities is not accessible
Wondering if we should nix the font-family definition & also move the 75px to 4em's.
#32
We need to nix the styles, all of them, no reason why the core module needs to supply styles + it's doing tricky things like height: 0; overflow:visible; as a positioning technique, which could be an accessibility issue with some screen-readers.
So:
1) We can't add a variable (fast API freeze and very close to beta), so add it in the template in D7 and fix in D8.
2) Styles are added by the theme in follow up issues, only needed for Bartik and Garland.
This is a bug:
a) use of color only is a WCAG violation
b) its major because there is no way for screen reader uses to get the same information on the same page, they will have to edit the node and review the publishing options - which is pretty unfair.
#33
OK, lets kill this in favor of #867830: "unpublished" status of rendered entities is not accessible, while this is older that issue is proposing a more robust solution.
#34