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

Mobile: Support building for web #10650

Open
wants to merge 142 commits into
base: dev
Choose a base branch
from
Open

Mobile: Support building for web #10650

wants to merge 142 commits into from

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Jun 22, 2024 •
edited
Loading

Summary

[ Try it | Forum post | Implementation progress ]

This pull request allows running Joplin Mobile in a web browser using react-native-web and @sqlite.org/sqlite-wasm.

Possible use-cases

  • Running Joplin on devices that can't easily install software.

  • Getting feedback on proposed mobile app changes — clicking on a link in a web browser is much easier than installing an Android APK.

  • Developing and testing plugins for mobile without access to an Android device.

  • Installing non-recommended plugins on iOS.

  • Could allow users to try plugins without installing them:

    • This would be done in a separate pull request. It could be done by hosting a separate copy of the web client, configured to use an in-memory database and virtual filesystem.

  • May simplify developing and testing the mobile app:

    • This could allow creating end-to-end tests for mobile with Playwright (we currently only have these for desktop).
    • On Linux and Windows, manually testing changes to the mobile app currently requires running an Android emulator or access to a physical Android device. On slower/lower-memory computers, Android emulators are very slow and make development more difficult.

Demo

The work/mobile-web branch is deployed to https://personalizedrefrigerator.github.io/joplin/web-client/.

Note that this demo works best in a Chromium-based browser. While it loads in Firefox, the initial setup is very slow.

Why not Joplin Desktop?

It seemed simpler to add support for web to the mobile app than the desktop app. In particular, Joplin Desktop depends on a large number of NodeJS-only libraries and APIs (e.g. fs-extra).

Tasks

  • Database
  • Note editor
  • Note viewer
    • Can view rendered markdown.
    • Mermaid
      • Mermaid scripts/styles aren't loaded.
    • KaTeX
      • Partial — KaTeX fonts aren't loaded.
    • External links
    • Internal links
  • Sync
    • Dropbox
    • File system sync (desktop Chromium-based browsers only).
    • Joplin Server — mostly works — see "Joplin Cloud".
      • Only works if served from the same origin as the web client.
    • Follow-up PR: OneDrive
      • Follow-up PR: Authentication workflow needs to be adjusted. For now, it's disabled.
    • Follow-up PR: Joplin Cloud
      • This can only be done after we decide on a hosting URL for the web client.
  • Drawing
    • Inserting
    • Editing
  • Allow installing plugins using "install from file"
    • Chrome
    • Firefox
  • Allow installing plugins from the plugin repo.
  • Camera.
    • Partial — only works on some mobile devices. On other devices, a file picker is shown.
  • Offline access & installability.
    • Currently, files are cached only when first requested. As such, if a font has never been requested (e.g. an icon font only used in settings), it might not be available offline.
  • Follow-up PR: Decrease size of bundle.js — extract plugin assets to separate files.
    • Optional — could be done separately. The bundle is currently about 14.5 MiB.
  • Fix accessibility bugs. Edit: On iOS, VoiceOver accessibility seems to work about as well as it does on mobile (has roughly the same issues). Focus issues seem to be similar.
    • Focus issues.
    • Pressing on a floating action button does nothing, when it should do something.
  • The last change to a note is sometimes not saved.
    • Observed in Google Chrome, may occur in other browsers.

@personalizedrefrigerator personalizedrefrigerator changed the title Proof of concept: Add web support to Joplin Mobile Proof of concept: Mobile: Support building for web Jun 22, 2024
@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Jun 29, 2024 •
edited
Loading

Remaining tasks before this pull request is ready for a review:

  • Improve the UI when an encrypted shared notebook can't be synced.
    • Currently, an orange "Press to set the decryption password." banner is shown (and the shared notebook remains encrypted).

I propose completing the following tasks in follow-up pull requests:

  • Setting up logic to deploy the web client to a hosting provider.
    • Probably do this in a new @joplin/joplin-web repository.
    • Publish hashes.
  • Compressing notes and plain-text resources before saving to the database.
  • Support for syncing encrypted shared notebooks.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review July 1, 2024 14:51
@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Jul 1, 2024 •
edited
Loading

This pull request is currently very large. I plan to move several changes from it into separate pull requests to make it easier to review. In particular,

  • Changing "s to 's in SQL queries (required by the sqlite-wasm version this pull request uses).
  • Dependency upgrades (e.g. redux)
  • New dialog provider logic on Android.
    • For now, this change doesn't apply to Android (due to style issues). Custom dialogs might be enabled on Android in a follow-up PR.

After the above tasks are completed, this pull request should be ready for review.

personalizedrefrigerator added a commit to personalizedrefrigerator/joplin that referenced this pull request Jul 1, 2024
@personalizedrefrigerator
Chore: Migrate SQL queries in preparation for web support
b9fa542 
This pull request's goal is to simplify laurent22#10650 by moving SQL
query-related changes into a separate pull request.
@personalizedrefrigerator personalizedrefrigerator mentioned this pull request Jul 1, 2024
Merged
@personalizedrefrigerator personalizedrefrigerator mentioned this pull request Jul 1, 2024
Closed
personalizedrefrigerator
personalizedrefrigerator commented Jul 2, 2024
View reviewed changes
packages/app-mobile/root.tsx
Comment on lines +634 to +637
// For now, geolocation is disabled by default on web until the web permissions workflow
// is improved. At present, trackLocation=true causes the "allow location access" prompt
// to appear without a clear indicator for why Joplin wants this information.
Setting.setValue('trackLocation', false);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, this is disabled by default because no user-visible reason is shown for why location permission is requested. The "Allow location access" prompt is shown when first creating a new note (after enabling location tracking), but there isn't any context for why Joplin wants to track the user's location.

Currently, however, I don't think the existing mobile app explains why location permissions are being requested. As such, for consistency, it could make sense to remove this different default:

Suggested change
// For now, geolocation is disabled by default on web until the web permissions workflow
// is improved. At present, trackLocation=true causes the "allow location access" prompt
// to appear without a clear indicator for why Joplin wants this information.
Setting.setValue('trackLocation', false);

personalizedrefrigerator
personalizedrefrigerator commented Jul 4, 2024
View reviewed changes
packages/app-mobile/components/screens/Note.tsx
Comment on lines +834 to +841
if (Platform.OS === 'web') {
const response = await pickDocument({ multiple: true, preferCamera: true });
for (const asset of response) {
await this.attachFile(asset, 'image');
}
} else {
this.setState({ showCamera: true });
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe refactor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@personalizedrefrigerator @laurent22