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

Update backstack record screen when returning value #295

Closed
wants to merge 22 commits into from

Conversation

kierse
Copy link
Collaborator

@kierse kierse commented Nov 21, 2022

This PR adds support for returning results from one screen to another, or - if an interceptor is set - to the activity or fragment managing the Circuit environment. This addresses #8.

Details

This PR adds several new concepts to assist returning values:

  1. ScreenResult - new marker interface that must be implemented by result objects
  2. ScreenResultHandler - used by screen presenters when returning a value
  3. ScreenResultInterceptor - optional callback (provided when calling NavigableCircuitContent) to intercept screen results for processing in an activity or fragment.
  4. ScreenReducer - new SAM interface that accepts a Screen and ScreenResult, and returns Screen?

How does it work?

To support situations where Circuit is running next to activities and fragments, there are two possible execution paths.

Screen-to-Screen

Child screen wishes to return a result to a parent screen, all within a single Circuit environment:

  1. A presenter invokes the ScreenResultHandler with a sub-class of ScreenResult
  2. Circuit grabs the parent backstack record and adds the result object to its args
  3. The child screen is popped off the backstack
  4. BasicNavigableCircuitContent:
    a. extracts the parent screen and pending result from the top backstack record
    b. attempts to create a new screen using any ScreenReducer's registered with the CircuitConfig
    c. recreates the backstack record using the new screen
  5. BasicNavigableCircuitContent updates the composition as before but now using the new screen instance (created at 4b) containing data from processed screen result

Screen-to-[Activity/Fragment]

The Activity or Fragment managing the Circuit environment wishes to intercept a specific ScreenResult sub-type

  1. When bootstrapping Circuit, the Activity/Fragment supplies a ScreenResultInterceptor callback when calling NavigableCircuitContent
  2. A presenter invokes the ScreenResultHandler with a sub-class of ScreenResult
  3. Circuit calls the provided ScreenResultInterceptor allowing the host to intercept results
  4. If the result is:
    a. intercepted, execution stops
    b. otherwise, execution mirrors step 4 above

Demo

To better demonstrate how both use cases work, I have added a new sample application at samples/navigation

navigator-demo.mp4

Remaining work

Some minor cleanup, KDocs and comments, tests, etc

@kierse kierse added the discussion Open discussions! label Nov 30, 2022
@kierse
Copy link
Collaborator Author

kierse commented Dec 5, 2022

Instead of adding an update(ScreenResult): Screen method to the Screen interface, another option to consider is adding a Map<Screen, ScreenReducer> to CircuitConfig (similar to what we do for presenter and UI factories):

fun interface ScreenReducer {
  fun reduce(result: ScreenResult: Screen
}

Users would only need to provide a reducer when bootstrapping Circuit if/when they expect one screen to return data to another

@kierse
Copy link
Collaborator Author

kierse commented Jan 5, 2023

Maintaining initial description here:

This PR is an experiment exploring a couple new approaches for returning results (#8).

  1. A new method on the Navigator: setScreenResult(ScreenResult?)
  2. Added a new interface called ScreenResultHandler who's sole purpose is to return data

Details
While the two approaches use a different API, they both save the returned data in a backstack record for processing once the current screen is popped.

Rough outline of what happens when a child screen returns a value to a parent:

  1. the child presenter invokes one of the above solutions for returning data
  2. both solutions grab the parent backstack record and add the result to the record args
  3. the child presenter invokes Navigator.pop, indicating it is finished
  4. BasicNavigableCircuitContent extracts the pending screen result and:
    a. recreates updates the screen (using new optional method on Screen)
    b. recreates the backstack record
  5. BasicNavigableCircuitContent updates the composition as before but now using the new screen instance (created at 4a) containing data from processed screen result

Both approaches have a number of benefits:

  1. screen results will survive a config change along with the backstack
  2. avoids weird/clunky solutions requiring the parent presenter to extract results from the navigator
  3. avoids having to add a nullable property to the navigator
  4. leans into concept of screens being presenter input state

Additionally, it should be trivial to build a custom interceptor (similar to what was done in #275) to intercept return values and route them to the containing Activity or Fragment for processing and use outside of Circuit.

Takeaways
There were 2 key takeaways for me from this experiment:

  1. I found recreating the screen to be an elegant way of feeding returned data to a receiving presenter
  2. using the backstack to store results has some benefits and avoids awkwardness commonly seen in other approaches

@kierse
Copy link
Collaborator Author

kierse commented Jan 5, 2023

@ZacSweers I circled back to this over the holidays and added a demo (as requested). There's still some more work to be done (cleanup, docs, tests, etc). Interested in your thoughts as I'm sure there is room for improvement.

cc @jpetote, @iamritav

@kierse
Copy link
Collaborator Author

kierse commented Feb 9, 2023

@ZacSweers and I chatted and agree that we don't have enough info yet to say for certain whether or not we actually need the ability to return data from one screen to another. Closing this until we find a compelling reason to revisit.

@kierse kierse closed this Feb 9, 2023
@ZacSweers ZacSweers deleted the ke/support-return-value branch February 9, 2023 21:50
@kierse kierse mentioned this pull request Feb 2, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussions!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant