add settings for reader screen margin#223
add settings for reader screen margin#223daveallie merged 12 commits intocrosspoint-reader:masterfrom
Conversation
|
Noting this may interact with #198 |
| {"Bookerly", "Noto Sans", "Open Dyslexic"}}, | ||
| {"Reader Font Size", SettingType::ENUM, &CrossPointSettings::fontSize, {"Small", "Medium", "Large", "X Large"}}, | ||
| {"Reader Line Spacing", SettingType::ENUM, &CrossPointSettings::lineSpacing, {"Tight", "Normal", "Wide"}}, | ||
| {"Reader Screen Margin", SettingType::ENUM, &CrossPointSettings::screenMargin, {"5", "10", "20", "40"}}, |
There was a problem hiding this comment.
Yeah it might be better in this case to have the actual values of 5, 10, 20, 40 be stored in the settings object instead of the enum options. It will mean it won't clash with changes to other existing enums. This is one of the first settings (besides the sleep timeout and refresh frequency) where the value is the setting.
Can you please create a new SettingType called VALUE and support setting values here. It may require some additional tweaks to SettingInfo to setup a few static constructors to make creating the settingsList a bit easier with all the different types.
|
FYI I also added this in #219 haha |
Where? I don't see any changes related to the reading screen margin in your PR, maybe Im missing something? |
Dave asked me to remove them for a cleaner PR |
|
Thanks for this, given the continued conflicts from this and other's PR as there are a lot of settings changes coming through and the structural changes to setting setup, I've stepped into resolve the conflict and make a few small tweaks to get it merged. |
* **What is the goal of this PR?** * This PR adds a setting to control the top left and right margins of the reader screen in 4 sizes (5, 10, 20, 40 pt?) and defaults to `SMALL` which is equivalent to the fixed margin of 5 that was already in use before. * **What changes are included?** * Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks, specific areas to focus on). --------- Co-authored-by: Dave Allie <[email protected]>
## Summary * **What is the goal of this PR?** * This PR adds a setting to control the top left and right margins of the reader screen in 4 sizes (5, 10, 20, 40 pt?) and defaults to `SMALL` which is equivalent to the fixed margin of 5 that was already in use before. * **What changes are included?** ## Additional Context * Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks, specific areas to focus on). --------- Co-authored-by: Dave Allie <[email protected]>
4 participants
Summary
SMALLwhich is equivalent to the fixed margin of 5 that was already in use before.Additional Context