Rearchitect BetterFeedback show>submit>pop Flow to Improve Dev Ex & Type Safety

[caseycrogers]

Is your feature request related to a problem? Please describe.

I'm taking user feedback that includes the following extra info:

1. User email (optional)
2. Referrer (what CTA/callback in the app directed the user to the feedback flow)

I also have a custom sheet that shows the user a spinner while their feedback is uploading and shows them an error message and allows them to retry if upload fails.

There are a bunch of parts of the API that I found made the requirements above hard to achieve:

1. extras is Map<String, dynamic> so I lose type safety between my feedback sheet and my database upload method (not a big deal for noSQL-very annoying for SQL)
2. My user's feedback details are split across UserFeedback's built in fields (text, screenshot) and extras so I have to merge them into a single serializable in the callback before sending to my DB.
3. show takes in a callback that is then passed through ~5 layers of redirection within the BetterFeedback source code. Makes the external API hard to use and the internal API confusing. Right now my onFeedback callback isn't being run and I'm having a really hard time tracing the code to figure out what I'm doing wrong.
4. My referrer string is just a dev-facing string telling me which CTA the user tapped on to activate the share flow. Right now there's no clear way for me to pass this to the feedback sheet and thus condition feedback sheet content/flow on it (and to get it to the DB I have to do the same awkward merging inside the callback that I have to do with screenshot.

Describe the solution you'd like

There are A LOT of moving pieces here with a bunch of different reasonable alternatives for solving each one. Here is the proposal that I think makes the most sense (while trying not to get bogged down in the alternatives). If you like this proposal we can refine it a little together and then I'm happy to write the PR to implement all of this.

The general idea is to make the feedback API mirror the Flutter Navigator 1 API as closely as possible. If you look at it closely, it becomes apparent that BetterFeedback is really just, control-flow-wise, a special case of Navigator. Here is the new API:

typedef FeedbackBuilder<T, R> = Widget Function<T, R>(
  BuildContext context,
  // All the callbacks the sheet needs to communicate with `BetterFeedback`.
  FeedbackFormController<T, R> formController,
);
...
// Manages the control flow from within the sheet builder.
class FeedbackFormController<T, R extends Object?> {
  // A generic object similar to navigator's routes that the dev can use to pass info to the feedback builder.
  // (example could be a "Referrer" enum that the feedback sheet can key behavior/user facing copy on.
 T route;

  // Same scroll controller as before, we just moved it into here.
  // Might be better just to leave it in the feedback builder callback-either way.
  ScrollController? scrollController;
  
  // Takes the feedback screenshot.
  // Much more flexible than the current system-this allows me to provide things like confirmation and
  // retake flows for the user as well as lets me include the screenshot in my own custom user feedback
  // object so that I don't have to piece it together later.
  Future<Uint8List> takeScreenshot() async { ... }

  // Closes the feedback form with the result object.
  // Similar to Navigator.pop but with more opinionated typing.
  // The result object holds the user feedback, but it is optional.
  // Note that it is up to the developer if they want to serialize and upload their result object from
  // inside the sheetbuilder (massively preferrable UX wise as you cans how loading and error
  // info to the user before closing the sheet...) or to simply return the result object and leave it 
  // to the caller to manage serializing and uploading it.
  void pop([R result]) { ... }
} 
...
class BetterFeedback<T, R extends Object?> {
  ...
  // `of` now takes type generics to force type safety.
  // If the dev needs fundamentally different feedback flows in their app with unrelated type signatures,
  // they can just put multiple `BetterFeedback` widgets in the tree and access them independently via
  // their respective type signatures.
  static FeedbackController<T, R> customFeedbackOf(BuildContext context) {...}
  ...
  static FeedbackController<void, UserFeedback> stringFeedbackOf(BuildContext context) =>
      customFeedbackOf<void, UserFeedback>();
  ...
  // Future now returns the user feedback as a typed generic.
  // In the default case this generic is `UserFeedback`.
  Future<R> show() {...}
  ...
  // This is the new dead-simple constructor that hides all the complexity and types everything
  // for you.
  BetterFeedback.stringFeedback({...}) : BetterFeedback.customFeedback<void, UserFeedback>({...});

  // This is the advanced constructor that allows you to deviate from the default types.
  BetterFeedback.customFeedback({...});
  
  // Perhaps the old default constructor is kept around for awhile as a migration period and redirects to the
  // new APIs under the hood.
  @deprecated
  BetterFeedback({...})
  ...
}

This is a pretty huge rewrite, but right now BetterFeedback is an awesome package that's tbh quite hard to use in any way but the default. This adds some up-front complexity (two generic types at the top level) but it makes the entire rest of the dev experience straightforward and type safe. By including a dead simple redirecting constructor, the basic case should remain as simple as it is today.

[ueman]

Hey, your proposals seem pretty reasonable to me 👍

[caseycrogers]

Awesome! I'll try to take a stab at a PR this weekend.