Replace Forum Icon Column with Topic Span

Great idea also I think we should change markup for headers for forum_list and maybe unify listings but this task for different issue.

Maybe "Topic type" more reasonable title for column? And better translatable

Let's get commiter's review. As for me it's ready

I don't really understand what 'Topic type' is -- can we think of different words to describe this? Ideally, it would be more explanatory.

@mgifford please separate "topic status" and "forum status". This confusion is all about the same icons that used for them.

If it's possible for D7 we should separate them and issue about #821672: Forum icons not accessible to screen-reader users

Really, I think that "Topic status" has more meaning, but OTOH this icon displays more "State"/Status for current user actually not "type"

I agree that 'Status' is better. I'd go with just 'Status' over 'Forum status'.

Another option would be 'Activity'. That might be a bit more explanatory than just 'Status'. Like 'Status', it is not perfect, but it provides more context and direction IMO.

I'm marking this 'needs review' so we can talk about it more (if necessary) and get a reroll.

Wait what? Why do we need a text there? Even for accessibility, the icon alt tags should be fine :S? Please we didn't just forget to place a label there, it was a conscious design decision.

Don't get to uncomfortable about empty boxes :), it really doesn't NEED an header.

Maybe better join this cells and type title with padding?
The empty cell looks really ugly, most of forums are make a padding for Icons

Are you sure that the problem caused by empty TH? I see no reason to change it to TD in THEAD section

@mgifford No offense, but a link with a very long document is very hard for me or anyone really to parse, so I would love these discussions to have some more grounding then "doesn't validate" ohh is needed for "accessibility".

I dont care so much for the table header as for the visual spacing - as long as we can retain that I am fine going with an alternative direction. But having a table header, that basically adds no value for visual users to me sounds rather silly trade off, if we can work around it - after all clarity and simplicity also comes from not-showing unnecessary information.

Not being someone with a cognitive or learning disability, I would imagine that having a visible header that somehow adds clarity to the content in the column may enhance accessibility.

@Bojhan

When you suggest that something was an intentional design decision can you please give some of the reasoning behind the decision.

If it adds no visual clutter, off to RTBC!

@mgifford I really do read the documents and spend quite some time learning about accessibility, trying out the different screen readers. However I am no programmer, I dont know html/css indepth to understand the implications and issues at hand with these very technical orientated documents. Hence my complaint that these documents are really hard to parse.

I felt you as an expert are far more capable to provide a short summary of the problem, without saying only that its an "accessibility" issue. When it comes to educating the community, I am sorry to say but linking off to "great" resources won't do it - I found the exact same with usability, there is a whole body of knowledge but to get your point across or expertise more often its all about providing a short insight in the accessibility/usability issue.

@Everett I am sorry, cant recall the issue that introduced this. But when we added these icons we saw there being a spacing problem putting it in its own <td> means we create a consistent visual space. However we should resist the urge to add description, titles and table headers to tables that only have icons - this because the icons themselves most of the time have more informational value then the header could ever provide. This case is an example of that, heaving a Status or Activity label would really not clarify the contents of that table, the actual icons (and possibly descriptions) do. Also I feel not having a header on this table creates some calm, as people don't have to reference to the table header and instead focus on the icon - this might sound weird, but its an automatic human behavior we are changing without confusing people.

I still sure that better to remove NULL and add 'colspan' => 2 to first header item and make css padding for first header cell.
This much better to be customizible for contrib where icons could have different width!

+++ modules/forum/forum.module	11 Aug 2010 21:10:04 -0000
@@ -867,7 +867,7 @@ function forum_get_topics($tid, $sortby,
-    NULL,
+    array('data' => t('Status'), 'field' => null, 'class' => 'element-invisible'),

1) Ugh. Does this really work in all browsers? I would have assumed that we'd need a SPAN to achieve that.

2) null should be NULL.

Powered by Dreditor.

um, to clarify, if that actually works in all browsers: awesome!

Bumping to D8, this can possibly be backported to 7.x.

1. There doesn't seem to be a consensus between the two proposed methods of solving the problem.
2. Sun's questions about how all browsers render the th when element-invisible has been applied has yet to be answered fully.

Still needs x-browser testing on this.

IMO this isn't a very critical issue, the alt on the image provides context.

I can test windows, what I am a I looking for - just XP and Windows 7, screens of the UI?

We recommend removing the icon column completely. Instead, we propose inserting contextual data inside the Topic cell within a span tag, such as <span>hot topic</span>. Using the CSS background-image property, the icon can then be displayed in that span.

This recommendation removes the extra column, adds context to the Topic data, and allows better Forum theming.

(via Montreal Accessibility Sprint).

idea is good, theming of forum should be changed

It is same to #1591744: Clean up CSS and markup for status report

But which apply to frontend, I afraid it will hurt SEO.

on search result page (google) will be:
hot topic this is an apple
hot topic this is a orange

when it is not a hot topic:
warning topic this is an apple
warning topic seo is bad

Not a good idea ?

Does screen reader read "ALT" or "Title" ?

I'm not an SEO specialist, but does Google really care about the forum listing page, or the unique content pages (articles / posts)?

@Everett Zufelt,
Not one know about it. (All SEO world trending to avoid any potential mistakes.)
my own experience, all listing pages have low value in search results.

Looking into the code, we already hided the texts in separated column / implement this pattern at forum listing page.

fix the display problem when there have long title topics (more than 2 lines).

forum-listing-better.patch removed DIV wrapper of $topic->created. maybe out of this issue scope. so im also attached min.patch.

Forum should change it's theme|preprocess and css more than this, see #1217002: Clean up the CSS for Forum module

+++ b/core/modules/forum/forum.cssundefined
@@ -15,8 +15,10 @@
-#forum td.forum .icon {
+#forum td .topic {

This should be changed to more short .forum .topic

+++ b/core/modules/forum/forum.cssundefined
@@ -15,8 +15,10 @@
+#forum td .icon{

I think core should not use direct tags.

+++ b/core/modules/forum/forum.cssundefined
@@ -31,24 +33,19 @@
-#forum .icon .topic-status-hot {
+#forum .icon.topic-status-hot {

is there a reason for .icon prefix at all?

#1217002 is still open. If you're agreed, I would like to fix CSS problem there. and this is mainly fixing accessibility issue. It is more easier to get things move forward.

Anyway, attached a new patch.

Makes a lot of sense, let's get review from one of accessibility guys! +1 to commit

My bad. I added same class to td & div. minimum the changes.

We think that there are an error with the path files in the patch #47.
We tried to change it

Sorry @mgifford, it's not a error.

I simply change the paths of the files containing the patch.

It looks like a case for IMG + ALT. But if there's a legitimate reason not to, we could just add sprites here.

Can we get a quick screenshot of the before/after visual changes with this patch?

Attached pre and post screenshots for the latest chrome, firefox and safari on osx for forum-listing-49.patch

updating status

Thanks @mducharme!

Per the summary, there are no visual changes (as the screenshots show), just an accessibility improvement. I think what we want is before/after markup?

Also, something is funky with the "Add new forum topic" link when the user doesn't have permission. A + button that says "You don't have permission to do this" doesn't make any sense. Let's file a followup for that.

Every screenshot shows the markup below the visual. Reverting status.

Looks like this still needs discussion.

Great, thanks!

Committed and pushed to 8.x!

Filed a followup issue as was requested in #60 #1853072: Remove forum_menu_local_tasks_alter() hack and instead add links in ForumController::build

Updated Issue Summary as per - http://core.drupalofficehours.org/task/1216