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.

Sign up for GitHub

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

Jump to bottom

Footer font size #5736

Merged
merged 4 commits into from Jan 4, 2020
Merged

Footer font size #5736

merged 4 commits into from Jan 4, 2020

Conversation

robert00s
Copy link
Contributor

@robert00s robert00s commented Jan 3, 2020 •
edited by Frenzie

Move footer font size (DMINIBAR_FONT_SIZE) from Advanced Settings to Status bar -> Settings

This change is Reviewable

@robert00s robert00s added this to the 2020.01 milestone Jan 3, 2020
@robert00s robert00s added the UX label Jan 3, 2020
@poire-z
Copy link
Contributor

poire-z commented Jan 3, 2020

Looks good to me.
May be just move the added menu item Font size above (before Sort items or after [ ] Show footer separator, before the separator ?), so the little less interesting Time format> and Duration format> both end up on the 2nd page?

@robert00s
Copy link
Contributor Author

Done :)
Moved after [ ] Show footer separator, before the separator

@poire-z poire-z merged commit 8a0ce98 into koreader:master Jan 4, 2020
@robert00s robert00s deleted the footer_font_size branch January 6, 2020 09:47
Frenzie
Frenzie reviewed Jan 28, 2020
View reviewed changes
frontend/apps/reader/modules/readerfooter.lua
width = Screen:getWidth() * 0.6,
value = font_size,
value_min = 10,
value_max = 18,
Copy link
Member

Choose a reason for hiding this comment

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

Is a max value technically required for this widget? (Not in a position to test atm.) See https://www.mobileread.com/forums/showthread.php?p=3946432#post3946432

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. max_value is required but I can prepare PR with value_min = 8 and value_max = 36

Copy link
Member

Choose a reason for hiding this comment

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

Alright, please do! 👍

@robert00s robert00s mentioned this pull request Jan 28, 2020
Merged
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
@robert00s @mwoz123
[UX] Footer: add option to set font size (koreader#5736)
d05ccf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Frenzie Frenzie Frenzie left review comments

Assignees
No one assigned
Labels
UX
Projects
None yet
Milestone
2020.01
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@robert00s @poire-z @Frenzie