theme_facetapi_link_inactive() and theme_facetapi_link_active() do similar things but parts of the code are in different order or achieve the same things in different ways.theme_facetapi_link_inactive() and theme_facetapi_link_active() construct their markup.theme_facetapi_deactivate_widget() but not a theme_facetapi_activate_widget()t('!facetapi_deactivate_widget !facetapi_accessible_markup') for active links (presumably so it can be re-ordered) but no corresponding one for inactive links.Clean up all these functions. Add a theme_facetapi_activate_widget(). Wrap the widgets in spans so they can be themed.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | make_inactive_active-2290031-7.patch | 6.01 KB | mparker17 |
Comments
Comment #1
mparker17Comment #2
mparker17Comment #3
mparker17I've attached a patch. Feedback welcome.
Comment #4
mparker17Noticed an incorrect function docblock.
Comment #5
cpliakas commentedHi mparker17.
Thanks for the contribution! I am all for improving accessibility as long as we are able to maintain some form of backwards compatibility.
Marking as needs work because it appears that patch no longer applies to HEAD.
Thanks!
Chris
Comment #6
mparker17Sorry for the delay in getting the re-rolled patch to you: it took me a little longer to verify that the re-rolled patch had the same result... apparently the merge conflict was due to #2166095: Inline documentation typo in facetapi.theme.inc, but that one-line-change threw off the diff algorithm and caused it to rearrange the whole patchfile. :P
Straight re-roll, so no interdiff.
This change shouldn't break backwards compatibility:
theme_facetapi_link_inactive()andtheme_facetapi_link_active()used to generate nearly-identical HTML in different ways. This patch fixes it so they generate the same HTML they did before, but in the same way. — EDIT: this is wrong.theme_facetapi_deactivate_widget()now puts an span around the(-)in active links so it can be themed. The active links will look the same, and the span won't show up unless you add CSS to make it do so.theme_facetapi_activate_widget()puts an empty span in inactive links (in the same place that the deactivate widget goes in active links) so it can be themed. The inactive links will look the same, and the span won't show up unless you add CSS to make it do so.Comment #7
mparker17Adding a class to the activate and deactivate links so it's easier to write CSS for them.
Comment #8
mparker17I was wrong. Active links now look like inactive links, i.e.: the format is
<a>{term} {accessible_markup} ({count}) {widget}</a>where the widget is an empty span with it's own theming function so it can be modified further down the theming layer.Comment #9
mgifford