feat(ONYX-986): support save and exit in new submission flow #10281

MounirDhahri · 2024-05-23T10:58:02Z

⟥⸻PR Description Up To Date [Friday 24th May 10:47 CEST] ⸻⟤

This PR resolves ONYX-986

Description

This PR makes the Save & Exit button in the new submission flow functional.

What we currently have:

The Save & Exit Button simply closes the modal or navigates you to the next screen
Expected behaviour:
When the user taps on Save & Exit, we should save their progress, and allow them to continue from where they left when they tap on Continue Previous Submission

Main challenges:

1. What data to save when the user taps on Save & Exit?

Data to inject for the submission
- Currently, I am saving the entire formik state and injecting it as it is, However, we can soon move toward saving only the submission id and querying for the data on screen mount. I didn't do that initially because we are going to get rid of this logic once we back up submissions with my collection artworks and have cross platform save and exit experience.
- Currently, we only store the submissionID and the last active step. We use the submissionID to query for the data
Navigation progress 👇 [see question 2]

2. How to restore the previous navigation state and get the user to continue from where they left

For that, we are leveraging initialState prop to inject the a partial navigation state. We are rebuilding it on mount and validating that the routes exist

Downsides of this approach

This works only for App. This is fine though since we want to first see how it performs when it comes to clicks before investing more effort.

Screen.Recording.2024-05-23.at.13.05.32.mov

PR Checklist

I have tested my changes on iOS and Android.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

support save and exit in new submission flow - mounir

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

ArtsyOpenSource · 2024-05-23T11:01:05Z

This PR contains the following changes:

Cross-platform user-facing changes (support save and exit in new submission flow - mounir)

Generated by 🚫 dangerJS against 51c046b

MounirDhahri · 2024-05-23T11:01:09Z

src/app/Scenes/MyCollection/State/MyCollectionModel.tsx

+  draft: {
+    values: ArtworkDetailsFormModel
+    currentStep: string
+    navigationState: any
+  } | null
+  setDraft: Action<this, this["draft"]>


Since we want to back the submission with my collection's artwork model, this should be a fine place to add it. We have a submission model that is powering the existing model, but it's currently so full of other stuff that doesn't make sense here and I couldn't use it just in case we roll back the FF.

olerichter00

I don't really understand what this PR is doing. Could you elaborate a bit on what "Save and Exit" means and how you've implemented it, please? The feature flag description says "Enable save and continue submission flow." which sounds more like what I understand from reading the code and looking at the screen recording.

MounirDhahri · 2024-05-23T12:33:59Z

Fair enough - I will add more details

olerichter00 · 2024-05-23T13:10:49Z

Fair enough - I will add more details

I just saw that you've added a description to the ticket. Now I understand what this PR is doing 💡

MounirDhahri · 2024-05-23T13:47:17Z

I just updated the description of the PR as well, I apologize again 🙏

olerichter00

Although this solution works, intuitively, I would say that this feature should rather be built on a RESTful edit route: /sell/submissions/:submissionID/edit. This URL could then accept a parameter to define the initial step the screen opens, such as ?initialStep=artworkDetails.

For the "Continue previous submission" button to work, the "Save and Exit" button could store the current step and submission ID on the device and make it accessible to link to /sell/submissions/:submissionID/edit?initialStep=:initialStep.

I'm happy to jump on a call to discuss these solutions 🤙

MounirDhahri · 2024-05-23T14:01:34Z

Your comment makes total sense - I think I can get to support that with not much effort. Let's do it together

MounirDhahri · 2024-05-24T12:27:57Z

@olerichter00 I continued on our progress yesterday, please let me know if there is something else to change.
Also, btw, we tested today's PR on today's build and it's working well

olerichter00

Looks great! 🌟

I've added a few comments here and there ✍️

olerichter00 · 2024-05-24T12:54:54Z

src/app/Scenes/MyCollection/State/MyCollectionModel.tsx

+  draft: {
+    submissionID: string
+    currentStep: string
+  } | null
+  setDraft: Action<this, this["draft"]>


Could we move this state somewhere else? It does not seem to be related directly to My Collection.

makes sense

olerichter00 · 2024-05-24T12:56:41Z

src/app/Scenes/SellWithArtsy/ArtworkForm/Components/SubmitArtworkTopNavigation.tsx

+      // Reset form if user is on the last step
+      // This is to ensure that the user can start a new submission
+      // This is not required but is a nice to have as a second layer of protection
+      GlobalStore.actions.myCollection.setDraft(null)


This is when the user has finished the submission, right? And in this case, we don't want to show the button anymore because there is no submission to continue...

yes exactly

olerichter00 · 2024-05-24T12:58:58Z

src/app/Scenes/SellWithArtsy/ArtworkForm/SubmitArtworkForm.tsx

+  // TODO: Better error handling
+  if (!data?.submission) {
+    return (
+      <Flex flex={1} alignItems="center" justifyContent="center">
+        <Text variant="sm-display">Submission not found</Text>
+      </Flex>
+    )
+  }


Could we use the "Unable to load" screen here?

eigen/src/app/Components/LoadFailureView.tsx

Line 17 in 942c557

export const LoadFailureView: React.FC<LoadFailureViewProps & BoxProps> = ({

olerichter00 · 2024-05-24T13:00:02Z

src/app/Scenes/SellWithArtsy/ArtworkForm/SubmitArtworkForm.tsx

@@ -45,6 +52,34 @@ export type SubmitArtworkStackNavigation = {
  ArtistRejected: undefined
 }

+export const SubmitArtworkFormEdit: React.FC<SubmitArtworkProps> = withSuspense((props) => {


What do you think about moving this into its own file? This file now exports two components.

sure - makes sense

olerichter00 · 2024-05-24T13:01:00Z

src/app/Scenes/SellWithArtsy/Components/Header.tsx


+  const showContinueSubmission = !!enableSaveAndContinue && !!draft?.submissionID


Suggested change

const showContinueSubmission = !!enableSaveAndContinue && !!draft?.submissionID

const showContinueSubmission = !!enableSaveAndContinue && !!draft?.submissionID

olerichter00 · 2024-05-24T13:01:49Z

src/app/Scenes/SellWithArtsy/SubmitArtwork/ArtworkDetails/validation.ts

+/**
+ * @deprecated
+ * Please use ArtworkDetailsFormModel from app/Scenes/SellWithArtsy/ArtworkForm/Utils/validation.ts
+ */


olerichter00 · 2024-05-24T13:03:23Z

src/app/Scenes/SellWithArtsy/ArtworkForm/SubmitArtworkForm.tsx

@@ -184,3 +272,17 @@ export const getCurrentRoute = () =>
  __unsafe__SubmissionArtworkFormNavigationRef.current?.getCurrentRoute()?.name as
    | keyof SubmitArtworkStackNavigation
    | undefined
+
+const getInitialNavigationState = (initialStep: SubmitArtworkScreen) => {


minor: This could get into a helper or util file to keep this file a bit smaller and clearer.

MounirDhahri requested review from nickskalkin and olerichter00 May 23, 2024 10:58

MounirDhahri self-assigned this May 23, 2024

artsy-peril bot added the Jira Synced label May 23, 2024

MounirDhahri commented May 23, 2024

View reviewed changes

MounirDhahri force-pushed the chore/add-save-and-exit-to-submissions branch 2 times, most recently from 69e805d to 17c3dad Compare May 23, 2024 11:08

olerichter00 reviewed May 23, 2024

View reviewed changes

MounirDhahri mentioned this pull request May 23, 2024

chore: add submission source to convection GraphQL artsy/convection#1462

Merged

MounirDhahri force-pushed the chore/add-save-and-exit-to-submissions branch 2 times, most recently from de72c62 to ea8241f Compare May 24, 2024 08:17

olerichter00 previously approved these changes May 24, 2024

View reviewed changes

MounirDhahri added 4 commits May 27, 2024 12:10

feat: support save and exit

1e6fb3a

fix: flaky test

6308e25

chore: update approach to use url routes

f7fccb7

chore: address review comments

51c046b

MounirDhahri dismissed olerichter00’s stale review via 51c046b May 27, 2024 10:26

MounirDhahri force-pushed the chore/add-save-and-exit-to-submissions branch from ea8241f to 51c046b Compare May 27, 2024 10:26

olerichter00 approved these changes May 27, 2024

View reviewed changes

MounirDhahri merged commit 34077fd into main May 27, 2024
7 checks passed

MounirDhahri deleted the chore/add-save-and-exit-to-submissions branch May 27, 2024 12:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ONYX-986): support save and exit in new submission flow #10281

feat(ONYX-986): support save and exit in new submission flow #10281

MounirDhahri commented May 23, 2024 •

edited

ArtsyOpenSource commented May 23, 2024 •

edited

MounirDhahri May 23, 2024

olerichter00 left a comment

MounirDhahri commented May 23, 2024

olerichter00 commented May 23, 2024

MounirDhahri commented May 23, 2024

olerichter00 left a comment

MounirDhahri commented May 23, 2024

MounirDhahri commented May 24, 2024

olerichter00 left a comment

olerichter00 May 24, 2024

MounirDhahri May 27, 2024

olerichter00 May 24, 2024

MounirDhahri May 27, 2024

olerichter00 May 24, 2024

olerichter00 May 24, 2024

MounirDhahri May 27, 2024

olerichter00 May 24, 2024

olerichter00 May 24, 2024

olerichter00 May 24, 2024

MounirDhahri May 27, 2024


		const showContinueSubmission = !!enableSaveAndContinue && !!draft?.submissionID

feat(ONYX-986): support save and exit in new submission flow #10281

feat(ONYX-986): support save and exit in new submission flow #10281

Conversation

MounirDhahri commented May 23, 2024 • edited

⟥⸻PR Description Up To Date [Friday 24th May 10:47 CEST] ⸻⟤

Description

What we currently have:

Main challenges:

1. What data to save when the user taps on Save & Exit?

2. How to restore the previous navigation state and get the user to continue from where they left

Downsides of this approach

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

ArtsyOpenSource commented May 23, 2024 • edited

This PR contains the following changes:

Choose a reason for hiding this comment

olerichter00 left a comment

Choose a reason for hiding this comment

MounirDhahri commented May 23, 2024

olerichter00 commented May 23, 2024

MounirDhahri commented May 23, 2024

olerichter00 left a comment

Choose a reason for hiding this comment

MounirDhahri commented May 23, 2024

MounirDhahri commented May 24, 2024

olerichter00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounirDhahri commented May 23, 2024 •

edited

ArtsyOpenSource commented May 23, 2024 •

edited