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

Font menu: add symbols for default and fallback fonts #3941

Merged
merged 7 commits into from May 11, 2018

Conversation

poire-z
Copy link
Contributor

@poire-z poire-z commented May 10, 2018

Adds symbols for default and fallback fonts.
Also allows for updating the fallback font and see results in real-time on the underlying document.
Also adds a menu item to generate a html document showing some sample text rendered with each available font.

image
(our default fallback font Noto Sans CJK has wrong glyphs for greek, so it's nice being able to see how others do - will need a base bump for koreader/crengine#186 for real-time update).

I recently came across https://poliseuse.wordpress.com/ (in french) which has reviews for many fonts and their use on ereaders - and I like testing fonts :)
I used to manually make some html document with the same text displayed with each font, so I can more easily (than with changing fonts with the menu) compare fonts.
With this new set of fonts to test, I thought that koreader could as well build that html document instead of me manually :)

So, this new menu item will display only if the user has a file koreader/settings/fonts-test-sample.html with some HTML snippet of his choice (*), and will generate a file in the home directory named fonts-test-all.html, that will allow comparing all fonts with the help of koreader features (Skim to widget, swipe to go to latest bookmarks...):
image
image

(*) dunno if this is a sufficiently interesting feature to have us add a default fonts-test-sample.html with some koreader blablah (sufficiently long so one can see paragraphs, wraps, line-height...), and some words/texts of various alphabets, CJK, links, superscripts, subscripts, numbers, italic... but if yes, feel free to write it :)

Also allows for updating the fallback font and see results
in real-time on the underlying document.
Also adds a menu item to generate a html document showing some
sample text rendered with each available font.
@NiLuJe
Copy link
Member

NiLuJe commented May 10, 2018

That reminds me that I should probably update our bundled fonts to the latest Noto release ;).

text = text .. " ★"
end
if v == fallback_font then
text = text.." ♻"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recycling? :-P

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much choice with what's available in Noto fonts (including the updated ones, seen no other symbols than the ones I'm used to).
test_unichars.html.txt : sample file where I do my icon shopping.

Copy link
Member

@NiLuJe NiLuJe May 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a (couple of) relatively small NotoSymbols fonts that we could pull if you fancy something in there... ;).

Getting the relevant menu entry to use them over the default UI font might get tricky, though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should be fine with what we have.
Btw, the greek glyphs have been corrected in your updated Noto Sans CJK, and they have also been included in the main Noto Sans, so good idea to update them :)

@poire-z
Copy link
Contributor Author

poire-z commented May 10, 2018

Alternate solution with less dark symbols (the blank square aiming at representing a missing glyph :):
image

@NiLuJe
Copy link
Member

NiLuJe commented May 10, 2018

I kinda prefer the filled, dark star.

Not quite convinced about the blank square, but I don't hate it ;).

@Eduardomb22
Copy link

I also prefer the dark star

@poire-z
Copy link
Contributor Author

poire-z commented May 11, 2018

As suggested by @Frenzie in #3942 (comment) :) and looks OK if we stay with filled/dark symbols:

image

@Frenzie
Copy link
Member

Frenzie commented May 11, 2018

The fallback character? That makes quite a bit of sense actually, but it was just a joke about the open book thing.

screenshot_2018-05-11_10-38-21

@poire-z
Copy link
Contributor Author

poire-z commented May 11, 2018

Oh, it displayed as that in my browser :)
image

@poire-z
Copy link
Contributor Author

poire-z commented May 11, 2018

I could update the message displayed when Hold, to somehow explain the symbols. Any preference for where the symbols should be (in the text or in the buttons)?
image

@Frenzie
Copy link
Member

Frenzie commented May 11, 2018

Probably in the text between parentheses.

Visually I prefer it in the buttons, but that might lead to space issues in some translations.

@poire-z
Copy link
Contributor Author

poire-z commented May 11, 2018

That would give this (with a non-break space before the opening parentheses, that isn't distinguished from a classic space in github code view):
image

@Frenzie
Copy link
Member

Frenzie commented May 11, 2018

Yes, I think that makes it clear that it's parenthetical information.

text = _("Generate fonts test html document"),
callback = function()
UIManager:show(ConfirmBox:new{
text = _("Would you like to generate a html document showing some sample text rendered with each available font?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an HTML document

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@poire-z poire-z merged commit a9905c5 into koreader:master May 11, 2018
@poire-z poire-z deleted the font_stuff branch May 11, 2018 15:48
poire-z added a commit to poire-z/koreader that referenced this pull request Aug 31, 2018
TouchMenu: added options to menu items with the following defaults:
    keep_menu_open = false
    hold_keep_menu_open = true
So, default for Tap callback is to close menu, and for Hold callback
to keep menu open.
In both cases, provide the TouchMenu instance as the 1st argument to
the callback functions (instead of a refresh_menu_func I added in koreader#3941)
so the callback can do more things, like closing, refreshing,
changing menu items text and re-ordering...

ReaderZooming: show symbol for default (like it was done for
ReaderFont, ReaderHyphenation...)
TextEditor plugin: update the previously opened files list in real
time, so the menu can be kept open and used as the TextEditor main
interface.
SSH plugin: keep menu open and update the Start/Stop state in real time
ReadTimer plugin: tried to do what feels right (but I don't use it)
poire-z added a commit that referenced this pull request Sep 4, 2018
TouchMenu: added options to menu items with the following defaults:
    keep_menu_open = false
    hold_keep_menu_open = true
So, default for Tap callback is to close menu, and for Hold callback
to keep menu open.
In both cases, provide the TouchMenu instance as the 1st argument to
the callback functions (instead of a refresh_menu_func I added in #3941)
so the callback can do more things, like closing, refreshing,
changing menu items text and re-ordering...

ReaderZooming: show symbol for default (like it was done for
ReaderFont, ReaderHyphenation...)
TextEditor plugin: update the previously opened files list in real
time, so the menu can be kept open and used as the TextEditor main
interface.
SSH plugin: keep menu open and update the Start/Stop state in real time
ReadTimer plugin: tried to do what feels right (but I don't use it)

Also remove forgotten cp in the move/paste file code
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

4 participants