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
Mobile: Settings screen: Create separate pages for each screen #8567
Mobile: Settings screen: Create separate pages for each screen #8567
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Henry, that looks very good.
-
Is it possible to use the exact same icons as the desktop app by any chance? If too complicated, like if we need to import another icon pack then don't worry about it. It would be good at least to have the Markdown section use the official Markdown icon.
-
In dark theme, is it possible to make the section description a bit brighter when it is currently selected?
-
I've also left a few comments regarding the section descriptions
Also thanks for refactoring several parts of the code to TypeScript!
packages/lib/models/Setting.ts
Outdated
'appearance': _('App theme, editor font'), | ||
'sync': _('Sync, encryption, proxy'), | ||
'joplinCloud': _('Joplin Cloud settings'), | ||
'markdownPlugins': _('Markdown plugins: Media player, math, footnotes, soft breaks, ...'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically those are plugins, but from a user perspective they are just options to turn on or off, so we don't need to mention "Markdown plugins". Maybe instead: "Media player, math, diagrams, table of content" (No "...")
packages/lib/models/Setting.ts
Outdated
'note': _('Geolocation, spellcheck, markdown toolbar, image resize'), | ||
'revisionService': _('Enable/disable note history, keep notes for...'), | ||
'tools': _('Application log, profiles, sync status'), | ||
'export': _('Export all notes'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Export your data" (since you can also export folders, tags, etc.)
packages/lib/models/Setting.ts
Outdated
'joplinCloud': _('Joplin Cloud settings'), | ||
'markdownPlugins': _('Markdown plugins: Media player, math, footnotes, soft breaks, ...'), | ||
'note': _('Geolocation, spellcheck, markdown toolbar, image resize'), | ||
'revisionService': _('Enable/disable note history, keep notes for...'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Toggle note history"?
packages/lib/models/Setting.ts
Outdated
'general': _('Language, date format'), | ||
'appearance': _('App theme, editor font'), | ||
'sync': _('Sync, encryption, proxy'), | ||
'joplinCloud': _('Joplin Cloud settings'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Email To Note, login information"
packages/lib/models/Setting.ts
Outdated
'sync': _('Sync, encryption, proxy'), | ||
'joplinCloud': _('Joplin Cloud settings'), | ||
'markdownPlugins': _('Markdown plugins: Media player, math, footnotes, soft breaks, ...'), | ||
'note': _('Geolocation, spellcheck, markdown toolbar, image resize'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"markdown toolbar" => "editor toolbar"
For some icons, we would need to add the Icomoon icon pack. I've made the icons that don't require this the same as on desktop: Note that the FontAwesome export icon is centered, but the arrow on the right (at least to me) makes it look off-center. |
Please find some observations based on the pre-release apk. Details I noticed:
Otherwise, moving settings under separate categories makes them much more approachable! Now, the settings hierarchy probably requires adjustments, as:
There are "tools", "export" and "more" categories whose respective purposes are not clear:
Things could be rearranged to better reflect how they relate to each other. For instance, it is stated in the doc that synchronisation settings are per profile. The settings hierarchy currently doesn't reflect that, as the synchronisation settings are directly attached to a top-level category, while profiles themselves are burried down below a "tools" category. Similarly, encryption settings should be shown (and maybe enforced) before synchronisation, otherwise it puts people's privacy at risk as their data will be synchronised without being encrypted first. If you agree with the general idea, maybe I can provide you with a refined hierarchy as an attempt to solve the issues listed above? |
Thank you for the feedback! This pull request currently uses the existing sections and settings and displays them in a different way. As such, it might make sense to include section/setting changes in a separarte pull request. The impact of many of the categorization issues could be improved by adding a search feature (which I would like to add soon).
I'm able to reproduce this in the settings screen before this pull request. As such, it isn't a new issue. I also don't see a way to have a smaller scale for smaller values and larger scale for larger values with the slider library we're currently using. Here are some ideas for fixing/improving this:
When I compile a release, I generally give it a different app ID and name from the main Joplin app (and the official Joplin beta releases). The goal is to make it easier to test a release without being forced to uninstall/reinstall the main application. It should show up as "Joplin (Beta)" on the device's home screen.
Some categories are difficult to update because they're shared with the desktop app. Changes made to these categories (and many of the settings in them) affect both clients. However, the "Tools", "Export", and "More information" categories are mobile specific, so should be much easier to re-organize without affecting the desktop app. Some notes:
As such, I propose getting rid of "Export debug report" entirely and merging its functionality into the "share" feature from the log. The current pull request is focused on converting the categories from headings into links on a separate page.
These settings are also in the "markdown" category in the desktop app (I think they don't apply to HTML notes, but I would need to check). With the current code structure, I expect it to be difficult to move them on mobile and not on desktop.
It might make sense to call this the "note editor toolbar" instead. It also includes "search" and "attach", which are arguably more general features. I do think it makes sense to have this near the "spellcheck" setting, so if we move this to a different category, it could make sense to move "spellcheck" as well.
This might be an artifact of the profiles feature being added relatively recently. It might make sense to move the "Encryption configuration" button to just below the sync target button (and perhaps call it "enable encryption" if encryption isn't enabled).
Note that most settings seem to be per-profile. For example, the app theme, the language, the time format, etc. |
Summary
Groups Joplin Mobile's settings screen into sections, where only one section is shown at a time.
In tablet mode, this design is similar to that of the desktop app.
Testing
Although this pull request has been marked as "ready for review", due to the large number of changes it makes to the settings page, a number of features need to be tested:
Filesystem sync (including choosing a new location)
Profile export
Sync report export
JEX export
Changing the theme
Attempting to exit the configuration screen with unsaved settings
Changing a slider setting
Changing a dropdown setting
Changing a text input setting
There are some VoiceOver issues -- accessibility focus doesn't go to the right place when switching screens. However, setAccessibilityFocus doesn't seem to be working (the title text component doesn't seem to be focusable, which might be related...). It might also be related to testing on a simulator and not a physical device.
I've built an Android pre-release with these changes (APK).
Screenshots
Screen recording
demo7.mp4