Fix onOutput ordering w/ Conflated Renderings Changes #876
Conversation
steve-the-edwards
force-pushed
the
sedwards/fix-output-ordering
branch
from
2eb893b to
9d70978
Compare
| public final class com/squareup/workflow1/ui/container/DialogOverlayKt { | ||
| public static final fun getOverlay (Landroid/app/Dialog;)Lcom/squareup/workflow1/ui/container/Overlay; | ||
| public static final fun getOverlayOrNull (Landroid/app/Dialog;)Lcom/squareup/workflow1/ui/container/Overlay; | ||
| } |
There was a problem hiding this comment.
@rjrjr What is missing in the rebase that apiDump removed this?
There was a problem hiding this comment.
Oh, I never checked that the down merge was green. I think that Rick upgraded our API checker and the rules changed. This looks legit.
There was a problem hiding this comment.
LMK if you want this fix in a separate commit.
There was a problem hiding this comment.
Its fine. Will just merge.
There was a problem hiding this comment.
Acutally. I'll do that other apiDump commit first.
There was a problem hiding this comment.
#877 for the API update.
steve-the-edwards
requested review from
a team and
zach-klippenstein
as code owners
steve-the-edwards
force-pushed
the
sedwards/fix-output-ordering
branch
from
9d70978 to
239e712
Compare
| // Only null will allow us to continue processing actions and conflating stale renderings. | ||
| // If this is not null, then we had an Output and we want to send it with the Rendering | ||
| // (stale or not). | ||
| while (actionResult == null) { |
There was a problem hiding this comment.
PTAL again @rjrjr as I changed the logic here such that if we ever had an Output we would emit the rendering and the Output so that we always satisfy the same constraints.
There was a problem hiding this comment.
Do we still need the enum? Seems like we're only checking is WorkflowOutput at this point.
There was a problem hiding this comment.
Good point. Though we may go back there. I'll leave the slight YAGNI in for now.
| // Only null will allow us to continue processing actions and conflating stale renderings. | ||
| // If this is not null, then we had an Output and we want to send it with the Rendering | ||
| // (stale or not). | ||
| while (actionResult == null) { |
There was a problem hiding this comment.
Do we still need the enum? Seems like we're only checking is WorkflowOutput at this point.
When adding in the option to conflate renderings, we changed the order in which the
StateFlowof renderings was updated and theOutputwas emitted.We return now to updating the rendering
StateFlowand then immediately callingonOutput.