Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved readibility #3280

Merged
merged 2 commits into from Apr 3, 2018
Merged

Improved readibility #3280

merged 2 commits into from Apr 3, 2018

Conversation

lmagniez
Copy link
Contributor

In order to solve the issue of having a good readibility on etherpad-lite, I changed as it was requested the set of user color (I made sure every color have a sufficient brightness), and changed the pad's font-size to 16px.
Issue's link: issue

@lpagliari
Copy link
Contributor

Awesome work, @lmagniez !! Thanks a lot for it!

@Dhwty does this PR impact your work, as you've mentioned on the original issue?

I'm still feeling these should be features enabled by props on settings.json -- one for the colors, one for the font-size / line-height... @lmagniez could you do that, please? The default style would be 12px / 16px, and the customised one 16px / 21px -- I'm trying to keep the same ratio for both configs here, not sure if this is the best approach. Thoughts?

If we use 16px / 16px, we won't have the spacing between lines, which make reading more comfortable.

12px / 16px:

image

16px / 16px:

image

16px / 21px:

image

@Dhwty
Copy link

Dhwty commented Nov 24, 2017

I'm still feeling these should be features enabled by props on settings.json -- one for the colors, one for the font-size / line-height...

If this change goes into settings.json, rather than being a hard-coded change in font size and colour, it's not going to impact me at all. Thanks for the @.

@lpagliari
Copy link
Contributor

we can define on settings.json.template that the improved styles would be used, and let other people avoid them if they want. At least we make more people to use the improvements of @lmagniez without they needing to do anything when starting their Etherpad instance for the first time

@vvalembois
Copy link

Sorry for the late reply, and thank you for your suggestions. We will work on it with @lmagniez and @lisonferez when we will be free.

@lpagliari
Copy link
Contributor

Thanks, @vvalembois . This will be an excellent improvement for Etherpad!

@JohnMcLear
Copy link
Member

I really dislike this PR. Sorry!

@JohnMcLear JohnMcLear closed this Apr 3, 2018
@JohnMcLear JohnMcLear reopened this Apr 3, 2018
@JohnMcLear
Copy link
Member

Seems the consensus is to implement this so merging.

@JohnMcLear JohnMcLear merged commit f15c7d7 into ether:develop Apr 3, 2018
@slel
Copy link
Contributor

slel commented Apr 4, 2018

Thaanks!!

@JohnMcLear
Copy link
Member

Had a number of complaints about this already...

  1. Line height wasn't bumped to match font-size
  2. I increased line height this has broken numbering and formatting.

@rohieb
Copy link
Contributor

rohieb commented Apr 11, 2018

As a short feedback to this PR, I have reverted it and related changes on my local instance for now, since I find 16pt too big to read and I cannot turn the font size back to 12pt otherwise except hard-reverting it. However I found that only on my Hi-DPI laptop, on the desktop it looked fine…

@muxator
Copy link
Contributor

muxator commented Apr 13, 2018

This change effectively breaks the alignment in Etherpad 1.6.4+ (see for example #3378).

As the feature seems interesting, maybe we can revert the changes on iframe_editor.css (I have them ready to commit) and open a separate line of work where these parameters would be configurable, as suggested by @lpagliari and others.

muxator added a commit that referenced this pull request Apr 13, 2018
This change partially reverts 0a9d025, which got released in 1.6.4
due to #3280.

Text size and line alignment are now reverted back to their 1.6.3
appearance (thus stay non customizable, for now).

Fixes #3378
@muxator
Copy link
Contributor

muxator commented Apr 13, 2018

Reverted back to old appearance with 9daade0.

We can build a more comprehensive approach in a different line of work.

@fungi
Copy link

fungi commented May 2, 2018

Any chance to get a new release for this sometime soonish? We're trying to stick to released versions in production deployments, but have gotten a number of complaints about the font size and line number misalignment since upgrading to deal with the recent vulnerabilities.

@muxator
Copy link
Contributor

muxator commented May 4, 2018

Released in 1.6.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants