2.1 main menu: various improvements for a11y, etc.

Started by Antechinus, September 08, 2015, 03:50:04 AM

Previous topic - Next topic

Antechinus

There is more than one issue here and they're interrelated, so I'm just going to lay them all out clearly rather than opening a pile of separate reports on GitUgh. If you lot decide you want to fix them, GitHub stuffz can come later.



The first issue is basic a11y. I assume you want SMF to hold its traditional market, which includes some government and organisational sites. These sites have regulations about accessibility that they have to follow, so they need software that provides good a11y. This includes the ability for users to access all menu functions, including access via Tab key and when using a screenreader, with or without javascript working. At the moment you don't have this, but it's easy to implement.

For sighted users there is currently no big problem when javascript is working. When javascript is not working for any reason (user disabled it, js error in site files, CDN offline, whatever) no drop menu content is accessible via the Tab key. The same applies to anyone using a screenreader. With javascript disabled, they will not be able to go through the drop menu links.

This is due to the use of display: none; on the relevant ul's in index.css (Line 880 in Beta 2). There's no need to use display: none; on the ul's. They will work perfectly if you use either left positioning, or a negative z-index, instead.

Yes, I know Joel White recommends display: none; with Superfish, but that's only because he's assuming people will want fancy closing animations. If your menu is doing animated stuff on mouseout, it will look silly whizzing off the left side of the page. If you use a z-index shift, that will cut off any other animations when it reaches a certain z-index. The recommendation to use display: none; has nothing to do with improved functionality, and it's bad for a11y. On that basis, I don't think it's a good idea for a default build.

Setting them up for all contingencies with left positioning does tend to be trickier than using z-index shifts, so the latter will probably make for easier implementation.



Oh yeah, the second issue is basic a11y too. :D If someone is on a screenreader, and if you change the display: none; in the CSS to either left positioning or negative z-index, then these guys will have no problem hearing the drop menu content when javascript is off. However, they'll still be screwed when javascript is on. This is because although your CSS will be sorted, jQuery and therefore Superfish, use display: none; to hide the drop menus.

This is easy to fix too, and really the same fix should be used wherever you are using javascript to set display: none; on any element (collapsible post options are another example). What you do here is use the very cool WAI-ARIA stuffz that was developed for this purpose. This validates with an HTML5 doctype and is well supported. It won't ever break anything. All you need is to have this on any li that has a drop menu under it:

aria-haspopup="true"
So you can do it like this:

<li id="button_', $act, '"', !empty($button['sub_buttons']) ? ' class="subsections" aria-haspopup="true"' :'', '>
There's another minor point here about indicators for drop menus for sighted users. In the Superfish plug-in it sets cssArrows: true, which would allow drop menu indicators if anyone set up some CSS for them. However, the SMF markup is already set up to allow drop menu indicators via the subsections class, and the subsections class has the advantage that it will work with or without javascript. So, in the case of SMF, all the cssArrows class is doing is cluttering up the markup for no benefit. It'd be better (although not critical) to set cssArrows: false, in either theme.js or smf_jquery_plugins.js.



Ok, next one: spans. You have a lot of empty spans strewn around in 2.1. Main menu is one place they lurk. Google doesn't like empty spans. Neither do I. :D

The generic_icons stuff in the main menu (and several other places) doesn't require markup full of empty spans. Pseudo elements are perfect for this stuff. If there are some cases where you do need a span to do the job you can still have the generic_icons class declared on spans, and you can even use the same basic CSS for the spans and the pseudos.

So you can lose all of this:

<span class="generic_icons ', $act, '" ', (!empty($button['custom']) ? ' ' . $button['custom'] : ''),'></span>', $button['title'], '
Change the CSS to this:

.generic_icons, #menu_nav>li>a:before {
    width: 16px;
    height: 16px;
    display: inline-block;
    background: url(../images/generic_icons.png) no-repeat -5px -5px;
    vertical-align: middle;
}

And change this:

.generic_icons.home {
    background-position: -135px -187px;
}

To this:

#button_home a:before{
    background-position: -135px -187px;
}

Voila. Oodles of spans gone. Markup easier to read. CSS no more complicated.



I can think of a couple more details too, if anyone is interested, but these are the main points.


Antes

clears the code yes, but also removes the ability to use those icons again (w/o adding extra into CSS).

Antechinus

You may need a little extra CSS here and there, but it's generally better to use CSS rather than markup.

Anyway, what about the other points?

Antechinus

Ok, here's an idea. You want to be able to re-use the same part of the sprite without increasing the amount of CSS? You can do that.

At the moment you are using two different class names for the same icon:

.generic_icons.administration, .generic_icons.home {
background-position: -135px -187px;
}


Presumably this is for the admin home page, and the main menu home button, and possibly a couple of other places. Ok, if it's just a home icon give it that class name. That way it can be used anywhere, without having to declare extra classes. And, not only that, but you can still get rid of all those spans in the main menu.

.home_icon, #button_home a:before {
background-position: -135px -187px;
}


This will work just fine, because the places outside the main menu that use this icon are all chained with .generic_menu in the markup. That means they'll naturally pick up the .generic_icon CSS you want for them, and then you just tweak the background positioning with the later declaration for .home_icon. Easy. Same thing can be done with the others. :)

Oldiesmann

So if we do class="generic_icons home_button", will it still work? All of the menu icons are handled via a sprite, which is why we have all those "generic_icons" sub-classes...
Michael Eshom
Christian Metal Fans

Antes

Quote from: Oldiesmann on September 08, 2015, 12:46:38 PM
So if we do class="generic_icons home_button", will it still work? All of the menu icons are handled via a sprite, which is why we have all those "generic_icons" sub-classes...

Yes, it 'll work but does it really worth creating that part of CSS again? I'm not sure about that. Since the icons are only for better visualization (we don't have standalone icons) a11y users won't be effected nor their browsing quality drop down.

Antechinus

Quote from: Oldiesmann on September 08, 2015, 12:46:38 PM
So if we do class="generic_icons home_button", will it still work? All of the menu icons are handled via a sprite, which is why we have all those "generic_icons" sub-classes...

I know they are handled by a sprite. Why do you think I was giving example CSS that includes sprite positioning? :D I have looked at how that class is used through all templates. It's a good idea on the whole, but I don't think the implementation is as good as it could be. It could be made cleaner, and less confusing for inexperienced themers. Up to you if you think that is worth implementing.

Regarding the class="generic_icons home_button" thing, that would work if you changed the anchor or li for that button to include the home_button class, and wrote the CSS to suit. You can make it work with any class name if you want to. It's only a little picture of a house. Call it class="little_picture_of_a_house" if you like and you can still make that work. It would work anywhere you wanted to use that icon.

At the moment the CSS is calling two different classes (.administration and .home) for exactly the same icon, which doesn't make much sense to me if you're aiming for concise CSS. Yes, I know it is being used as a home icon on the main menu and the admin home page, but there's still no reason for that to have two different classes. You could use .generic_icons.home for both instances and save some code.




It's probably a bit late at this stage to do what I think is the best idea, which would be to use numbering to indicate which portion of the sprite you were using. You have 100 icons, and they all have names in English, and those names might not be immediately obvious to everyone. Also, the positioning is done on a 26px grid, which is an awkward number for anyone using a base 10 number system.

If you used a 25 or 30px grid instead, the numbers would make more sense at a glance (easier mental addition). If you did the icon CSS so that the first one was class="gi_1", meaning "number 1 icon on the generic icons sprite", and numbered the rest of them from gi_2 through to gi_100, the class name would immediately make it obvious which part of the sprite was being called. You wouldn't have to read names in English. You'd just have to be able to count.




Anyway, probably a bit late for that particular idea. I don't think there's any problem in ditching the superfluous spans though. It's easy to do. SMF 2.0.x was full of superfluous spans, and anyone who has worked with 2.0.x probably has a severe aversion to them. :D

I do think the other points I raised about a11y are well worth implementing. They seem to be getting overlooked in all the kerfuffle about sprites.

Antechinus

Oh christ, I just noticed the latest version of this on GitHub has added even more superfluous spans: https://github.com/SimpleMachines/SMF2.1/blob/release-2.1/Themes/default/index.template.php

Honestly, this is  just building bloat on bloat now.

<li id="button_', $act, '"', !empty($button['sub_buttons']) ? ' class="subsections"' :'', '>
<a', $button['active_button'] ? ' class="active"' : '', ' href="', $button['href'], '"', isset($button['target']) ? ' target="' . $button['target'] . '"' : '', '>
', $button['icon'],'<span class="textmenu">', $button['title'], '</span>
</a>';


So now you have $button['icon'] which is going to output a .generic_icons span, then you have the .textmenu span as well.

If you are going to change it, it would make far more sense to just have a declaration for .generic_icons:before in the CSS, and then set the .generic_icons class and its associated classes straight on the anchors.

Far less markup to generate. Far cleaner CSS. Seriously. Doing it this way would not require you to rewrite all the .generic_icons CSS either. It would still be just as reusable. All you'd need is a markup cleanup wherever the old spans are spewed out on an unsuspecting internet, and a slight CSS tweak in a couple of places.

If you want a menu without visible text in the links, just use text-indent: 100%; white-space: nowrap; and overflow: hidden; on the anchors.

Doing it this way has another advantage, in that it's still good for a11y if the title attribute happens to be empty.

Yes, it's that simple. Basic CSS. Use it. :D

live627

While going through these old topics, I noticed this ol thing. What needs done here? I'll assume everything, since this has gone unnoticed/unresolved. Annoying.

Antechinus

#9
To be honest: at this stage of the game I have no idea what, if anything, still needs doing. This was over four years ago. I don't know where the 2.1 code is up to at the moment. I'd have to go through it again and refresh my memory. I can grab the latest nightly and take a look.

ETA: Oh joy. Latest nightly has borken upgrade.php. :P

SychO

It's not that it has been forgotten, I for one haven't forgotten about this, it's just that there is currently a good number of theme related pull requests still waiting to be merged that it is difficult to make more theme related pull requests, at least for me.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

Fair enough. I got a test site upgraded to the latest nightly anyway, after a bit of messing around. Only had to rebuild the whole thing a couple of times. :D

Seems to be a bug with registration settings with the install and upgrade scripts at the moment. Screws the database structure if you try to set tighter than default registration settings (meaning if you try to set registration to disabled when installing, it will completely wipe out all db tables apart from two stray ones).

Advertisement: