Skip to content

Flutter controller v2#464

Open
isha382 wants to merge 82 commits into
ob-f:masterfrom
3dwesupport:flutter-controller-v2
Open

Flutter controller v2#464
isha382 wants to merge 82 commits into
ob-f:masterfrom
3dwesupport:flutter-controller-v2

Conversation

@isha382
Copy link
Copy Markdown
Contributor

@isha382 isha382 commented Nov 25, 2024

This is having new changes in ui for flutter controller which is now having a setting icon in which there is option to change the controller mode and some other ui changes.

sparsh3dwe and others added 30 commits December 15, 2022 11:43
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need those?

@thias15
Copy link
Copy Markdown
Collaborator

thias15 commented May 8, 2026

Sorry for the very long delay on this review. Submitted my pending comment above; here's the broader feedback:

  1. Dead images. docs/images/app_controller_settings_1.jpg and app_controller_settings_2.jpg aren't referenced anywhere in the README — only flutter_controller_setting.jpg is. Were they intended to illustrate something that got dropped?
  2. Deprecated import android.app.Fragment in SharedPreferencesManager.java. The codebase otherwise uses androidx.fragment.app.Fragment, and the import looks unused (only the string return type from getFragment() is involved). Could you remove it?
  3. Enum naming convention. Enums.fragmentType should be FragmentType to match the existing SensorType, LogMode, ControlMode.
  4. Persistence layer that does nothing. SharedPreferencesManager.setFragment() is called in each fragment's onViewCreated and immediately read back via getFragment() to emit the event. It's never used across launches — should this just be a local variable (or have the fragment type passed directly to createFragment)?
  5. Removed "double-tap to go back" behavior in controlSelector.dart. The old code had onDoubleTap callbacks on TiltingPhoneMode/OnScreenMode to flip back to mode selection. The new code wraps the same widgets in GestureDetector without an onDoubleTap, so the gesture detectors are dead code. Was the intent to drop the gesture in favor of the new menu drawer? If so, the empty GestureDetector wrappers can go too.
  6. Protocol inconsistency. Existing messages use {"status": {"<NAME>": "<value>"}}. The new fragment-type message uses a top-level {"FRAGMENT_TYPE": "<value>"}, so the Flutter side now has to branch on two shapes. Could FRAGMENT_TYPE be nested under status like the rest, to keep the protocol uniform?
  7. Conflicts with Dependency updates to latest versions. #507. That PR also touches Flutter Android Kotlin (it goes to 2.1.0 vs 1.7.0 here) and the iOS Info.plist scene-manifest. Whichever lands first will force the other to rebase. Worth coordinating with the Dependency updates to latest versions. #507 author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants