Comments

attheshow’s picture

Assigned: Unassigned » attheshow
Status: Active » Needs review

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.

attheshow’s picture

StatusFileSize
new2.85 KB
moshe weitzman’s picture

A screenshot would help move this along. Sounds proper to me.

attheshow’s picture

StatusFileSize
new73.28 KB

Good idea. Thanks. Screenshot is attached.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

well done.

catch’s picture

Agreed, very nice.

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs design review

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.

yesct’s picture

Just checking...this would be translatable, yes?

catch’s picture

Yes, the unpublished is run through t().

johnalbin’s picture

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.

yoroy’s picture

Yes much better of course. The only way to solve this unambiguously is with actual text.
Any reason to still keep the pink then?

modulist’s picture

A background of diagonal lines would also get the job done. Think of the background patterns on checks.

dave reid’s picture

@modulist: I don't understand how diagonal lines would say that content is not published...

yoroy’s picture

Status: Needs review » Reviewed & tested by the community

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.

yoroy’s picture

Status: Reviewed & tested by the community » Needs work

Eh sorry. Let's remove the background color first, it's meaningless.

attheshow’s picture

Status: Needs work » Needs review
StatusFileSize
new1.95 KB

Removed the background color. Attaching updated patch.

webchick’s picture

New screenshot, please?

attheshow’s picture

StatusFileSize
new275.59 KB

Sure thing. Attached.

sign’s picture

Works fine, one minor thing, do we have to show UNPUBLISHED on node preview too?

Bojhan’s picture

Probally not.

sign’s picture

Added three lines to node.css

.node-preview div.unpublished {
  display: none;
 }

works fine

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

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.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)

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

Jeff Burnz’s picture

Status: Closed (fixed) » Needs work
Issue tags: +Needs accessibility review

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?

mgifford’s picture

Issue tags: +accesssibility

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

yoroy’s picture

mgifford’s picture

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.....

mgifford’s picture

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.

mgifford’s picture

Looking at the related issue #867830: "Unpublished" style of rendered entities is not accessible (and looks bad)

Wondering if we should nix the font-family definition & also move the 75px to 4em's.

Jeff Burnz’s picture

Category: task » bug
Priority: Normal » Major
Status: Needs work » Needs review
StatusFileSize
new2.17 KB

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.

Jeff Burnz’s picture

Status: Needs review » Closed (duplicate)

OK, lets kill this in favor of #867830: "Unpublished" style of rendered entities is not accessible (and looks bad), while this is older that issue is proposing a more robust solution.

johnalbin’s picture

Category: bug » task
Priority: Major » Normal
Status: Closed (duplicate) » Closed (fixed)
avpaderno’s picture

Component: node.module » ajax system
Issue summary: View changes
Issue tags: -accesssibility +accessibility
avpaderno’s picture

Component: ajax system » node system
Issue tags: -accessibility +Accessibility