wearethoughtfox / amnesty-twitter

MIT License
0 stars 0 forks source link

Tidy up #8

Open robertocarroll opened 6 years ago

robertocarroll commented 6 years ago
robertocarroll commented 6 years ago

PDW testing.

The “Not on Twitter? Email them instead!” button

  1. In Internet Explorer and Edge, this seems to retain some browser default

button-ie-bug.png

I was able to remove them by adding style="font:inherit; background:none; color:inherit;" to the button. You might want to do this in the stylesheet.

  1. In Edge and Chrome on Android, the +/- icon got a focus outline the the button was tapped/clicked on.

I was able to remove this by adding style="outline:none;" to the element. You might want to do this in the stylesheet.

  1. In IE 9–11, the +/- icon makes itself 640 pixels wide, due to IE’s different handling of SVG scaling.

I was able to fix this by adding style="width:1rem;" to the , to match the existing height value. However, this gets trickier with the text issue.

  1. At smaller viewport widths, the +/- icon looks a little misaligned vertically.

button-narrow.png

I think this is because its height is set to 1rem, i.e. the same as the browser’s base font size. This is fine when the button’s text size is bigger than that (as it is on wider viewports), but perhaps a bit big when the button’s text size is smaller.

I’m not sure whether there’s a Tachyons class you can add to the SVG to fix this, or whether you’d write some CSS targeting the same viewport sizes as Tachyons does.

Curly apostrophes

  1. The curly apostrophes are written as opening single quotes (ALT+], ‘).

apos.png

They should* be written as closing single quotes (ALT+SHIFT+], ’).

*This is a bit of a rabbit hole — if you’d like to fall down it, see:

Dynamic stuff

  1. I notice Twitter’s about page has the letters in the first section animated in with a sort of typing effect, and that the down arrow can be clicked on to scroll the page down a bit.

Your page doesn’t yet have that. That’s probably not a problem, but I thought I’d flag it just in case. (If it is a requirement I can help if needed.)

Validation errors

  1. There’s a couple of small validation errors:

    a) Tell Twitter to make this stop (missing / from closing span tag)

    b)

  2. About
  3. (Only

  4. can be a child of
      . The error comes from Twitter themselves, but you can improve on their shoddy coding by adding the dn and db-l classes to each of the subsequent
    • s, instead of on a invalid )

      c) href="https://twitter.com/intent/tweet?text=Put the text of the tweet here: http://link via @twitter"

      Technically, the spaces should be URL-encoded. This doesn’t seem to cause a problem though, so I think it’s fine to leave it like this for easier editing by Amnesty.

Colour contrast

  1. The #ToxicTwitter text in the green and purple sections, and the white text over the Laura Bates photo, don’t look like they have adequate colour contrast for accessibility. Obviously this would require design rework, and might be fine in context.

Footer text width

  1. Really tiny one this, but at a viewport width of 1024 (which is the iPad Pro in portrait, hence me spotting it), the footer text (“2018 Amnesty International”) wraps onto two lines, which seems unnecessary.

All fixed!