[feat] Dynamically Load Themes in AppFlowy

[a-wallen]

Feature Preview

dynamic_theme_demo.mp4

Loads themes from a JSON file at runtime.

TODO

• Run build_runner in flowy_infra to pass CI
• Fix InvalidType build_runner errors.
• Provide default values or throw an error if FlowyColorScheme API changes and the old theme is incompatible with the new API.
• Load the themes from a different directory (for example, path_provider getApplicationDocumentsDirectory()).
• Allow users to drag and drop themes via the GUI (in the appearance settings menu).

AppFlowy.2023-06-06.20-56-36.mp4

• Delete uploaded themes.

AppFlowy.2023-06-09.12-05-53.mp4

• Better prompt messages
• Better error messages
• Fix all merge conflicts from the recent merge of develop into main
• Refactor (code style)
• Widget/integration test for previous use cases.
• Users should be able to browse available JSON themes on a website.

Issues #1070. It does not fix but makes it easier to experiment with new themes.

---

PR Checklist

• My code adheres to the AppFlowy Style Guide
• I've listed at least one issue that this PR fixes in the description above.
• I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
• All existing tests are passing.

[a-wallen]

If the FlowyColorScheme API changes. The parser will provide transparent as the default value.

Another way to fail silently is to pass over dynamic themes that can't be parsed. I.e., they won't be included if the app is updated since the theme is invalid. I thought this would be the best way to fail silently.

[appflowy]

How about FlowyDynamicPlugin?

[a-wallen]

Note: after adding json_annotation, json_serializable, and build_runner from appflowy's pubspec to flowy_infra's pubspec causes 3 different versions of the dart analyzer to download to the pub cache. Since the InvalidType error suggests that the Locale type is unresolved, the issue is probably coming from a broken analyzer. The current main @71d118904871d5526c13479ca2e672d4dd81b6ea downloads analyzer version 5.2.0 and _fe_analyzer_shared-50.0. Adding the same packages to flowy_infra downloads analyzer 5.13 and _fe_analyzer_shared-61.0.0.

[codecov]

Codecov Report

Merging #2670 (b3b99a5) into main (2c513be) will decrease coverage by 0.52%.
The diff coverage is 31.53%.

@@            Coverage Diff             @@
##             main    #2670      +/-   ##
==========================================
- Coverage   67.28%   66.77%   -0.52%     
==========================================
  Files         409      414       +5     
  Lines       19502    19664     +162     
==========================================
+ Hits        13122    13130       +8     
- Misses       6380     6534     +154     

Flag	Coverage Δ
appflowy_flutter_integrateion_test	64.67% <31.15%> (-0.56%)	⬇️
appflowy_flutter_unit_test	12.06% <14.23%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...wy_flutter/lib/plugins/document/document_page.dart	70.49% <ø> (ø)
...ation/editor_plugins/base/link_to_page_widget.dart	73.91% <0.00%> (ø)
...ditor_plugins/header/custom_cover_picker_bloc.dart	61.53% <ø> (ø)
...ion/editor_plugins/image/image_selection_menu.dart	19.04% <ø> (ø)
...gins/document/presentation/share/share_button.dart	91.66% <ø> (ø)
...er/lib/user/presentation/folder/folder_widget.dart	42.34% <ø> (ø)
..._flutter/lib/workspace/application/appearance.dart	84.74% <0.00%> (-0.18%)	⬇️
...tion/home/menu/app/header/import/import_panel.dart	83.33% <ø> (ø)
...widgets/settings_file_customize_location_view.dart	84.53% <ø> (ø)
...ettings/widgets/settings_file_exporter_widget.dart	1.60% <ø> (ø)
... and 14 more

... and 4 files with indirect coverage changes

[a-wallen]

Working on testing some of the plugin infrastructure but FlowyDynamicPlugin.decode is making it a bit hard for me. I will resume tomorrow.

[appflowy]

Would it be better if we moved this widget to flowy_infra_ui and made it a configurable widget? Then it could be used for theme upload and file upload in the database.

[a-wallen]

I will hold off on this because I'd like to keep all of the theme upload widgets together. Some of the theme upload widgets use LocaleKeys which requires AppFlowy.

[annieappflowy]

Can we aim to release the MVP by the end of next week?

[a-wallen]

Sure 👍

[a-wallen]

AppFlowy.2023-06-29.12-18-23.mp4

[Xazin]

I'm looking at this and I can't help but think if there is a better way to do it.

The only way I could think of introduced ~15 more lines of code though, not sure it's worth it.

~25 lines of code version

    final srcAsList = src.listSync();

    File? light;
    File? dark;

    for (int i = 0; i < srcAsList.length; i++) {
      if (light != null && dark != null) {
        break;
      }

      final event = srcAsList[i];
      if (event is File) {
        if (p.basename(event.path).contains(lightExtension)) {
          light = event;
        } else if (p.basename(event.path).contains(darkExtension)) {
          dark = event;
        }
      }
    }

    if (light == null || dark == null) {
      throw PluginCompilationException(
        'The theme plugin does not contain both a light and dark version: `theme.light.json` and `theme.dark.json`.',
      );
    }

[a-wallen]

At least this way, we're only listing the directory once. This is a good review comment. Let me try to expand on this.

[a-wallen]

Haha @Xazin, if we're code golfing it then you can try to beat this :)

    final files = src.listSync().whereType<File>().toList();

    File? light, dark;

    for (int i = 0; i < files.length || (light == null && dark == null); ++i) {
      final file = files[i];
      if (p.basename(file.path).contains(lightExtension)) {
        light = file;
      } else if (p.basename(file.path).contains(darkExtension)) {
        dark = file;
      }
    }

[a-wallen]

Either way we still get the benefit of only listing the directory once.

[Xazin]

Yeah that is indeed better. Semantically it's the same, but more readable.