Opened 3 years ago

Closed 2 years ago

#4514 closed defect (fixed)

Font readability on acceptableads.com

Reported by: juliandoucette Assignee: ire
Priority: P2 Milestone:
Module: Websites Keywords:
Cc: christiane, p.pastourmatzis, martin, saroyanm, jeen, athornburgh, ire Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29497585/

Description (last modified by juliandoucette)

Environment

  • Desktop web browsers

How to reproduce

  • See any page containing several consecutive paragraphs e.g. users

Observed behaviour

I find it hard to read main text on acceptableads.com. My eyes strain and I make more mistakes than usual.

I think this is because of:

  • font family
  • font weight
  • line height

Expected behaviour

Increase:

  • Body text line height increased from 1.2 to 1.5
  • Button font size increased from 12px to 16px
  • Button height increased to 44px (text inside to be centrally aligned)
  • Footer text to be min 14px (It's12 px currently)

Attachments (3)

Screen Shot 2016-10-10 at 18.43.05.png (93.5 KB) - added by juliandoucette 3 years ago.
font-improvement.pdf (48.7 KB) - added by juliandoucette 3 years ago.
issue29374562_29374563.diff.txt (11.0 KB) - added by juliandoucette 3 years ago.
Patch from code review 29374562.

Download all attachments as: .zip

Change History (29)

Changed 3 years ago by juliandoucette

comment:1 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:2 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:3 Changed 3 years ago by juliandoucette

  • Cc jeen added

Has there been any movement on this Jeen?

comment:4 Changed 3 years ago by jeen

The font family has been chosen for its wide language support, is extremely versatile for the range of weights it offers and renders well at small sizes.

I think simple increasing the line-height of the text (both body text and title text) from 120% to 150% would improve readability a lot.

@christiane: would you be alright with this change?

comment:5 Changed 3 years ago by juliandoucette

From @christiane:

Up all font styles by 2px. Up h1 by 4px and increase line height to 150%

Changed 3 years ago by juliandoucette

comment:6 Changed 3 years ago by juliandoucette

  • Priority changed from Unknown to P2
  • Ready set

comment:7 Changed 3 years ago by juliandoucette

  • Owner set to juliandoucette

comment:8 Changed 3 years ago by juliandoucette

  • Cc athornburgh added

comment:9 Changed 3 years ago by juliandoucette

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:10 Changed 3 years ago by juliandoucette

  • Priority changed from P2 to P3
  • Ready unset

@saroyanm I'm reducing the priority of this change based on a discussion with @athornburgh and because I think that the solution that is in review right now needs some more work.

Changed 3 years ago by juliandoucette

Patch from code review 29374562.

comment:11 Changed 3 years ago by juliandoucette

  • Review URL(s) modified (diff)

I think we should also take another look at the spacing in sections within the scope of this issue e.g. the accent section on the users page.

comment:12 Changed 3 years ago by juliandoucette

  • Status changed from reviewing to reopened

comment:13 Changed 3 years ago by jeen

Following up on this. I would like the following updates made (not sure if this can all go in one ticket):

  • Body text line height increased from 120% to 150%
  • Button font size increased from 12px to 16px
  • Button height increased to 44px (text inside to be centrally aligned)
  • Footer text to be min 14px (It's12 px currently)

comment:14 Changed 3 years ago by juliandoucette

  • Owner juliandoucette deleted

comment:15 Changed 3 years ago by juliandoucette

  • Priority changed from P3 to P2

comment:16 Changed 2 years ago by ire

  • Cc iaderinokun added

I think the suggestions made by @jeen here work.

@juliandoucette Is there more you would like to do?

comment:17 Changed 2 years ago by juliandoucette

@ire I think what Jeen suggested will work. If not, let's address it in another issue.

comment:18 Changed 2 years ago by christiane

where exactly is the problem here?

comment:19 Changed 2 years ago by juliandoucette

  • Description modified (diff)

where exactly is the problem here?

I've updated the description. Sorry for the confusion christiane. You suggested a solution to this problem a long time ago but it hasn't been implemented yet because of other priorities.

comment:20 Changed 2 years ago by ire

  • Cc ire added; iaderinokun removed

comment:21 follow-up: Changed 2 years ago by ire

@juliandoucette: @jlow: Can we begin implementing this?

comment:22 in reply to: ↑ 21 Changed 2 years ago by jeen

Replying to ire:

@juliandoucette: @jlow: Can we begin implementing this?

Yes please!

comment:23 Changed 2 years ago by ire

  • Owner set to ire
  • Ready set

comment:24 Changed 2 years ago by ire

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

comment:25 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 4514 - Font readability on acceptableads.com

comment:26 Changed 2 years ago by ire

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.