heydon / revenge.css Goto Github PK

Hey Heydon!

First of all, I love the idea. I think it does a great job showing what's not great in a web page with CSS only however I think it's a bit too agressive for someone who justs want to make sure he didn't do something awful.

I'd like to see several levels of warnings like:

• smooth: not great but okay,
• okay: not a good idea,
• bad: mostly a bad idea,
• agressive: omfg? how dare u?.

I think using Sass would make things easier. For instance, we could have a mixin accepting a warning level or something, that would then dump only the rules for this level and below.

Then a Compass extension, a npm package or anything would make it very cool to use. Like:

@import "revenge.css";
@include revenge('agressive'); // or whatever like this

Just my 2 cents. I like what you did so far. :)

Update: having a closer look at the code, I think we even could have warning levels like aria, a11y, semantics, etc.

https://www.forkliftsmi.com

<a class='fl printHide' href='/'><img id='Logo' alt='Forklifts of Michigan' src='/images/logos/ForkliftsMichiganLogo.png'/></a>

Like when using Font Awesome

<i class="fa fa-facebook fa-2x"></i>

Hi @Heydon

I made a few changes in a forked version and if you get a spare moment would appreciate your comments.

https://github.com/Decrepidos/REVENGE.CSS

Added or removed the following:

Added not:span.fa and i.fa to exclude Font Awesomeicons hooks on empty elements.

Added i.fa:not([aria-hidden="true"])::after, span.fa:not([aria-hidden="true"])::after {
content: 'Use aria-hidden="true on icons.' !important;
} to make sure aria-hidden is included on icons

Removed as

/*
body > div#footer:not([role="contentinfo"])::after, body > div.footer:not([role="contentinfo"])::after {
content: 'Use role="contentinfo" on this footer element or use native HTML5

/***
Added if div has class or ID of header but is not a native element include role. Considering if .banner #banner should be included
*/
div#header:not([role="banner"])::after, div.header:not([role="banner"])::after {
content: 'Use role="banner" on this header element or use native

https://heydonworks.com/revenge_css_bookmarklet/ renders a 404:

Arrived here from @brucelawson's 10/12/19 post (https://www.brucelawson.co.uk/2019/checklist-to-avoid-the-most-common-accessibility-errors/) where he recommends this bookmarklet for checking certain common accessibility errors.

Similar to the issue he logged (#25), is the “Forms must have action attributes” still relevant?

HTML5 spec https://www.w3.org/TR/html52/sec-forms.html#element-attrdef-submitbuttonelements-formaction:

The action and formaction content attributes, if specified, must have a value that is a valid non-empty URL potentially surrounded by spaces.

'If specified' as in 'optional'? This answer on StackOverflow seems to think so (https://stackoverflow.com/questions/9401521/is-action-really-required-on-forms). According to them it was required in HTML4 but not in HTML5 and can be left off. So if doctype is HTML5 there is no need for the action attribute?

.. pretty sure this is obsolete in today's browsers? (Ditto contentinfo etc). Happy to make a PR ..

(and perhaps show a message on any main that is not the only/ first main on a page, because 8.06% of pages erroneously contain more than one main landmark (https://almanac.httparchive.org/en/2019/accessibility#main-landmark)

Hi Heydon,

I love this and have had a lot of fun using it on my website and my competitors (yes, I get a real kick out of seeing how many HTML fuck-ups they've made)

What I really think the project needs however is some explanation as to why some things are bad.

I imagine where a rule isn't considered best practices, you still have some very logical reasons, but it would be useful to know them.

For example, I am quite puzzled about why I often see If you are going to make it look like a button, make it a button, dammit! on <a class="button"> elements. These 'buttons' are hyperlinks and should work without JavaScript and be seen as links by search engines and screen readers. Does that mean they should not look like buttons? I think that's a very harsh limitation to a designer.

Thanks for your time and contributions to the web community.
Tom

I find the sr-only class used for off-screen screen reader only text used in sites. Often it is used unnecessarily. For example: <h2 class=”sr-only”> This is the site header</h2>.
I recommend including the following to flag this.
.sr-only:after{
content: 'Off-screen text for screen reader users. Is this text necessary?' !important;
}

Since you don't aim at supporting IE 8 based on the number of :not() occurrences, I think you could move all the :after to ::after to stick to the specifications.

No big deal, just a minor improvement.

One of your error messages is:

Do not use list items to divide up lists unless they have role = separator

However <li role="separator"> is invalid per http://www.w3.org/TR/html5/dom.html#sec-implicit-aria-semantics :

• Language feature: li element whose parent is an ol or ul element
• Restrictions: If specified, role must be one of the following: listitem, menuitem, menuitemcheckbox, menuitemradio, option, radio, tab, treeitem or presentation

Note how separator is absent from that list.

Perhaps presentation should be recommended instead?

I'd love to see some indication of which version I've got stored on my bookmarks bar, compared to the current master version. Is it possible to add a version near the beginning of the bookmarklet code?

i.e.
javascript:(function(){'1.0.0';revenge=document.createEleme...
or
javascript:(function(){version='1.0.0';revenge=document.createEleme...
?

I realise that the bookmarklet is just a way to link in the .css, but it still wouldn't hurt to have a version present.

I believe there is a rule matching all correctly formated images:

img[alt]:not([alt=""])

This targets all img tags with an alt attribute which is not empty. So... Every properly formated image as far as I know. I believe you want to target:

img[alt=""]

... but you might want to override this rule for images in figure, because it's common practice to empty the alt attribute when there is a figcaption.

As far as I know, pseudo-elements do not work on replaced content. img is a replaced element, thus the ::after pseudo-element won't display anything.

More informations: http://red-team-design.com/css-generated-content-replaced-elements/.

Sure this style predates HTML5 and HTML Living Standard, so hard to criticize, but nowadays it is allowed to have non-nested DIV elements inside DL:

https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element:concept-element-content-model

Also AFAIK it newer was a mistake to have multiple definitiondescription therms defscribed by one or more DD.

So such sample should suffer no revenge:

<dl>
  <div>
   <dt>foo</dt>
   <dt>bar</dt>
   <dt>baz</dt>
   <dd>Very common metasyntactic variables.</dd>
   <dd>Frowned-upon words without meaning often seen in programming handbooks.</dd>
  </div>
  <div>
    <dt>metasyntactic variable</dt>
    <dd><q cite="https://en.wikipedia.org/wiki/Metasyntactic_variable">specific word or set of words identified as a placeholder in computer science and specifically computer programming</q></dd>
  </div>
</dl>

I like this!
Any change of adding this to the npm repository, or at least add a package.json file in the github repo (so that one can install it via npm)?