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

Mobile: Implement plugin screen redesign #10465

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented May 24, 2024

Summary

This pull request partially implements a redesign of the plugin screen.

This implementation is based on a Figma design by @MartaLC.

Notes

  • To simplify the implementation, many of the default React Native Paper styles were used in place of the ones in the Figma design.
    • This commit reverted some style customizations that made buttons and other controls closer to the original design, but led to more complicated code.
  • This pull request makes slight changes to the colors used by React Native Paper widgets in other parts of the app.
    • For example, the main button color changed from black/dark gray to the blue shown in the below screenshots.

To-do

  • Remove mostly-unrelated refactoring changes from this pull request. See:
  • ☐ (optional) Add more automated tests.
  • ☐ (optional/UI) Make accordion views closer to those in the Figma design?
    • The original design uses separate screens while this pull request uses accordions.
    • Joplin Mobile currently has a few bugs/issues that make separate screens potentially more difficult to use:
      • Back button doesn't go to the correct tab in settings.
      • On tablet devices, the settings sidebar isn't visible.
  • ☐ (optional/UI) Improve lack of feedback when installing from a file?
    • Currently, after selecting a plugin with "install from file", there is no clear UI indicator that the plugin installed successfully unless "Installed plugins" is open.

Screenshots

Search and installed plugins in light mode:
4 screenshots. From left to right: Search results for the letter "A", now shows result count, renders installed plugins differently; Installed plugins list, buttons are hidden until clicked; plugin dialog, now has enabled/about/update which were previously only on each plugin card; Search again, with a different search query

Deleting a plugin from the plugin management dialog, no update available:
4 screenshots

Advanced settings in light mode and in dark mode (below a search):
2 screenshots

On a large-screen device:
3 screenshots: Shows installing Quick Links then opening its dialog. The settings screen sidebar is visible until the plugin management dialog is opened.

Note: The "on a large screen device" screenshot was updated after reducing the maximum width of the info dialog.

@personalizedrefrigerator personalizedrefrigerator added mobile All mobile platforms plugins Anything related to Joplin's plugin system labels May 24, 2024
@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review May 27, 2024 13:12
@personalizedrefrigerator personalizedrefrigerator changed the title Mobile: Partially implement plugin screen redesign Mobile: Implement plugin screen redesign May 27, 2024
testID='searchbar'
placeholder={_('Search')}
mode='outlined'
left={<TextInput.Icon icon='magnify' />}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This currently causes a warning when the component is mounted:

Warning: TextInput.Icon: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.

This upstream pull request, when merged, should remove the warning.

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Jun 3, 2024

I've just found a bug where the version number doesn't update in the plugin modal after installing an update. I'm currently fixing this and writing a test case. Edit: Resolved.

Comment on lines -145 to 153
public waitForLoadedPluginsChange() {
return new Promise<void>(resolve => {
this.pluginsChangeListeners_.push(() => resolve());
});
public addLoadedPluginsChangeListener(listener: ()=> void) {
this.pluginsChangeListeners_.push(listener);

return {
remove: () => {
this.pluginsChangeListeners_ = this.pluginsChangeListeners_.filter(l => (l !== listener));
},
};
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously, waitForLoadedPluginsChange (added to support mobile plugins), was used to wait for changes to the list of loaded plugins in a while loop. For example,

while (!event.cancelled) {
   await service.waitForLoadedPluginsChange();
   ... update something ...
}

This, however, has the drawback that waitFor listeners can't be cancelled when the plugin unloads. It's instead necessary to wait for the listener to be fired, before it can be removed. Additionally, it's necessary to call waitForLoadedPluginsChange from async code — the exhaustive dependency eslint task doesn't seem to check the dependencies of useAsyncEffect.

Because addLoadedPluginsChangeListener is now used in more places (usePlugin), it has been refactored.

@laurent22 laurent22 merged commit 06f42e8 into laurent22:dev Jun 4, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile All mobile platforms plugins Anything related to Joplin's plugin system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants