Use only one library `.dart` file in `sharezone_widgets` package. #538 #543

Marc-R2 · 2023-03-31T17:14:47Z

Description

All files in lib/sharezone_widgets/lib/[...].dart that contained only exports were merged into one sharezone_widgets/lib/sharezone_widgets.dart and the individual export files were removed.

Note:
There were two duplicate widgets PlaceholderModel && PlaceholderWidgetWithAnimation implemented in both app/lib/homework/shared/placeholder_templates.dart and lib/sharezone_widgets/lib/src/placeholder.dart.
This caused an import conflict as both were imported due to the bundling. Since the widgets were (almost) identical, I merged them into lib/sharezone_widgets/lib/src/placeholder.dart and removed them from app/lib/homework/shared/placeholder_templates.dart (and the associated private widgets were also moved).

Related Tickets

Closes #538

…le to correct spelling

…one_widgets package SharezoneApp#538

docs-page · 2023-03-31T17:14:51Z

To view this pull requests documentation preview, visit the following URL:

docs.page/sharezoneapp/sharezone-app~543

Documentation is deployed and generated using docs.page.

CLAassistant · 2023-03-31T17:14:52Z

All committers have signed the CLA.

Jonas-Sander

Hey thanks for your PR! :)

Looks good so far, although there are some things that still need to be changed:

Your auto-formatting seemed to have mode some changes to Strings inside our code (see the example I commented on below). For this PR I would like to not make these kind of changes. If possible please revert these changes.
Under [...]/sharezone_widgets/lib there are still the files animated_visibility.dart, expanded_chip.dart, sharezone_widgets.dart, wrapper.dart. These can be moved to sharezone_widgets/lib/src and also be exported inside sharezone_widgets/lib/sharezone_widgets.dart.
I get the error that The imported package 'flare_flutter' isn't a dependency of the importing package. Try adding a dependency for 'flare_flutter' in the 'pubspec.yaml' file.
I don't know why this wasn't an error before, but it needs to be fixed anyways. So this means that flare_flutter: ^3.0.2 needs to be added to the sharezone_widgets/pubspec.yaml (this is the same version that is in the app/pubspec.yaml)
There are several lints that are shown in my IDE. These need to be fixed, otherwise our pipeline will fail.

Analyzer lints:

sharezone_widgets (lib/sharezone_widgets):
  error - Target of URI doesn't exist: 'package:flare_flutter/flare_actor.dart' at lib\src\placeholder.dart:9:8 - (uri_does_not_exist)
  error - The method 'FlareActor' isn't defined for the type '_Rive' at lib\src\placeholder.dart:219:14 - (undefined_method)
  info - The import of '../../dialog_wrapper.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\adaptive_dialog\left_and_right_action_dialog\left_and_right_action_adaptive_dialog.dart:15:8 - (unnecessary_import)
  info - The import of '../adaptive_dialog_action.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\adaptive_dialog\left_and_right_action_dialog\left_and_right_action_adaptive_dialog.dart:16:8 - (unnecessary_import)
  info - The import of 'long_press.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\adaptive_dialog\long_press_dialog\long_press_adaptive_dialog.dart:15:8 - (unnecessary_import)
  info - The imported package 'flare_flutter' isn't a dependency of the importing package at lib\src\placeholder.dart:9:8 - (depend_on_referenced_packages)
  info - Constructors for public widgets should have a named 'key' parameter at lib\src\placeholder.dart:18:9 - (use_key_in_widget_constructors)
  info - Use 'const' with the constructor to improve performance at lib\src\placeholder.dart:88:26 - (prefer_const_constructors)
  info - The import of 'package:sharezone_widgets/src/state_sheet/state_dialog_content.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\state_sheet\dialog.dart:11:8 - (unnecessary_import)
  info - The import of '../dialog_wrapper.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\state_sheet\dialog.dart:14:8 - (unnecessary_import)
  info - The import of 'state_sheet_content.dart' is unnecessary because all of the used elements are also provided by the import of 'package:sharezone_widgets/sharezone_widgets.dart' at lib\src\state_sheet\sheet.dart:12:8 - (unnecessary_import)
  
sharezone_app (app):
  info - Sort directive sections alphabetically at lib\blackboard\details\blackboard_details.dart:28:1 - (directives_ordering)
  info - Unused import: 'package:sharezone/homework/shared/placeholder_templates.dart' at lib\pages\homework\homework_details\submissions\homework_create_submission_page.dart:17:8 - (unused_import)

Jonas-Sander · 2023-03-31T20:32:31Z

app/lib/groups/src/pages/course/course_card.dart

-      content: Text(
-          'Möchtest du den Kurs "$courseName" wirklich endgültig löschen?\n\nEs werden alle Stunden & Termine aus dem Stundenplan, Hausaufgaben und Einträge aus dem Schwarzen Brett und gelöscht.\n\nAuf den Kurs kann von niemanden mehr zugegriffen werden!'));
+    context: context,
+    right: AdaptiveDialogAction.delete,
+    defaultValue: false,
+    title: "Kurs löschen?",
+    content: Text(
+      'Möchtest du den Kurs "$courseName" wirklich endgültig löschen?\n\n'
+      'Es werden alle Stunden & Termine aus dem Stundenplan, Hausaufgaben '
+      'und Einträge aus dem Schwarzen Brett und gelöscht.\n\n'
+      'Auf den Kurs kann von niemanden mehr zugegriffen werden!',
+    ),
+  );


I guess this was done by some auto-formatting on your end? This change was made in several places in this PR.

I mean it's not a bad change per se but as we don't use that kind of String formatting currently I would be more comfortable if you could revert these kind of changes in this PR.

Jonas-Sander · 2023-03-31T20:52:06Z

Some actions (should be android-integration-test, ios-integration-test, macos-build-test, web-preview) currently will always fail for external PRs, so don't be confused (#348)

…idgets/pubspec.yaml

…rapper.dart to sharezone_widgets.dart bundle

Marc-R2 · 2023-03-31T22:38:16Z

Thanks for the feedback.

I moved the remaining files to src/ and added them to the bundle accordingly, and worked on the lints.

I also tried fixing the flare dependency. I suspect this was related to the duplicate widgets mentioned above and the corresponding pubspec.lock (please check - otherwise I'll have to think about it again).

I am still working on reverting the strings. This may well be a result of my formatter, as I find strings more readable that way, and the autoformat key is quickly pressed - I apologise

Jonas-Sander

Only two more minor changes needed, otherwise it LGTM! :)

Jonas-Sander · 2023-04-01T12:50:35Z

.gitignore

 # IDE
+
+## VS Code
 .vscode/*
 !.vscode/launch.json
 !.vscode/settings.json

+## Android Studio / IntelliJ
+.idea/
+
 # FVM will create a relative symlink in your project from .fvm/flutter_sdk to


Please revert since it has nothing todo with the goal of this PR.

Jonas-Sander · 2023-04-01T12:52:05Z

app/pubspec.yaml

@@ -74,7 +74,6 @@ dependencies:
  firebase_messaging: ^14.2.5
  firebase_performance: ^0.9.0+14
  firebase_storage: ^11.0.14
-  flare_flutter: ^3.0.2


Don't remove it here. We need it both in app/pubspec.yaml and lib/sharezone_widgets/pubspec.yaml.

This reverts commit 061d902.

# Conflicts: # app/lib/donate/page/donate_page.dart # app/lib/navigation/models/navigation_item.dart

Jonas-Sander

LGTM! 💙🎉

Jonas-Sander · 2023-04-01T20:46:07Z

@nilsreichardt How should we go about merging this? Just force-merge? Might the merge queue cause problems?

nilsreichardt

LGTM

nilsreichardt · 2023-04-02T12:33:00Z

lib/filesharing/files_usecases/lib/src/file_viewer/widgets/file_page_app_bar.dart

@@ -54,7 +54,10 @@ class _Title extends StatelessWidget {
  Widget build(BuildContext context) {
    return Text(
      name,
-      style: const TextStyle(color: Colors.white, fontFamily: rubik),


Just a note for future PRs: Would be great to only have changes in a PR that fit the purpose of the PR. This purpose of the PR is to refactor the imports of the sharezone_widgets package and should not contain any formatting changes like this.

I mean I guess thats the same kind of auto-formatting that would happen if we save the file again to edit the imports ourselves, no? So I don't know if thats the case I don't have a problem with it really 🤷‍♂️
Or is it different formatting from ours?

No, in this case a comma has been added manually after fontFamily: rubit and then it was formatted

nilsreichardt · 2023-04-02T13:02:34Z

@nilsreichardt How should we go about merging this? Just force-merge? Might the merge queue cause problems?

Yes, just force merging. Force merging also skips the merge queue.

Jonas-Sander · 2023-04-02T21:13:46Z

Alright, thanks again @Marc-R2 :)

Marc-R2 added 4 commits March 31, 2023 16:46

chore(.gitignore): Add IDE files and directories to ignore list

061d902

refactor(adaptive-dialog): fix typo in import statement and rename fi…

9f09f5d

…le to correct spelling

refactor(sharezone_widgets): add all exports to single file in sharez…

17c7be8

…one_widgets package SharezoneApp#538

refactor: Remove separate export files now in sharezone_widgets.dart

fdedc3e

Marc-R2 requested review from Jonas-Sander and nilsreichardt as code owners March 31, 2023 17:14

github-actions bot added user: pupil / student user: teacher labels Mar 31, 2023

Marc-R2 changed the title ~~Use only one library .dart file in sharezone_widgets package. #538~~ Use only one library .dart file in sharezone_widgets package. #538 Mar 31, 2023

Jonas-Sander requested changes Mar 31, 2023

View reviewed changes

Marc-R2 added 2 commits April 1, 2023 00:14

fix(pubspec): Update dependencies in app/pubspec.lock and sharezone_w…

f7c23a5

…idgets/pubspec.yaml

refactor(bundle): add animated_visibility.dart, expanded_chip.dart, w…

f96f26e

…rapper.dart to sharezone_widgets.dart bundle

github-actions bot added dependencies Changing, updating, adding or removing one or more dependencies. legal Regarding Licenses, Policy updates, Warnings to users (that might cause trouble if not there) etc. labels Mar 31, 2023

revert(Strings): Undo changes in String linebreaks from 9f09f5d

a77e91c

Jonas-Sander requested changes Apr 1, 2023

View reviewed changes

Marc-R2 added 3 commits April 1, 2023 15:54

Revert "chore(.gitignore): Add IDE files and directories to ignore list"

6653ff4

This reverts commit 061d902.

revert(pubspec): re-add flare to app/pubspec.yaml

84e005c

Merge branch 'main' into SharezoneApp#538

c09053e

# Conflicts: # app/lib/donate/page/donate_page.dart # app/lib/navigation/models/navigation_item.dart

Jonas-Sander approved these changes Apr 1, 2023

View reviewed changes

nilsreichardt approved these changes Apr 2, 2023

View reviewed changes

Merge branch 'main' into SharezoneApp#538

cbe8c7a

Jonas-Sander merged commit 4c8e57f into SharezoneApp:main Apr 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use only one library `.dart` file in `sharezone_widgets` package. #538 #543

Use only one library `.dart` file in `sharezone_widgets` package. #538 #543

Marc-R2 commented Mar 31, 2023 •

edited by nilsreichardt

Loading

docs-page bot commented Mar 31, 2023

CLAassistant commented Mar 31, 2023 •

edited

Loading

Jonas-Sander left a comment •

edited

Loading

Jonas-Sander Mar 31, 2023

Jonas-Sander commented Mar 31, 2023 •

edited

Loading

Marc-R2 commented Mar 31, 2023

Jonas-Sander left a comment

Jonas-Sander Apr 1, 2023

Jonas-Sander Apr 1, 2023

Jonas-Sander left a comment

Jonas-Sander commented Apr 1, 2023

nilsreichardt left a comment

nilsreichardt Apr 2, 2023

Jonas-Sander Apr 2, 2023

nilsreichardt Apr 3, 2023

nilsreichardt commented Apr 2, 2023

Jonas-Sander commented Apr 2, 2023

Use only one library .dart file in sharezone_widgets package. #538 #543

Use only one library .dart file in sharezone_widgets package. #538 #543

Conversation

Marc-R2 commented Mar 31, 2023 • edited by nilsreichardt Loading

Description

Related Tickets

docs-page bot commented Mar 31, 2023

CLAassistant commented Mar 31, 2023 • edited Loading

Jonas-Sander left a comment • edited Loading

Choose a reason for hiding this comment

Jonas-Sander Mar 31, 2023

Choose a reason for hiding this comment

Jonas-Sander commented Mar 31, 2023 • edited Loading

Marc-R2 commented Mar 31, 2023

Jonas-Sander left a comment

Choose a reason for hiding this comment

Jonas-Sander Apr 1, 2023

Choose a reason for hiding this comment

Jonas-Sander Apr 1, 2023

Choose a reason for hiding this comment

Jonas-Sander left a comment

Choose a reason for hiding this comment

Jonas-Sander commented Apr 1, 2023

nilsreichardt left a comment

Choose a reason for hiding this comment

nilsreichardt Apr 2, 2023

Choose a reason for hiding this comment

Jonas-Sander Apr 2, 2023

Choose a reason for hiding this comment

nilsreichardt Apr 3, 2023

Choose a reason for hiding this comment

nilsreichardt commented Apr 2, 2023

Jonas-Sander commented Apr 2, 2023

Use only one library `.dart` file in `sharezone_widgets` package. #538 #543

Use only one library `.dart` file in `sharezone_widgets` package. #538 #543

Marc-R2 commented Mar 31, 2023 •

edited by nilsreichardt

Loading

CLAassistant commented Mar 31, 2023 •

edited

Loading

Jonas-Sander left a comment •

edited

Loading

Jonas-Sander commented Mar 31, 2023 •

edited

Loading