Generalize goBack feature for CollectionCreate

[zecakeh]

Instead of relying on the name of the previous screen to send the newly created colUid back, we use a route param. This will allow to have the same behavior on other screens.

Fixes #156

Tested on web Firefox Linux Tested on native Android

[tasn]

This feels more hacky to me. I'd rather avoid complexity as much as possible. What does this PR actually solve that's worth the added complexity?

[zecakeh]

To me it's a UX improvement. There are two ways you can create a Collection:

• From the drawer: after you create the collection, you'd expect to be redirected to the newly created collection
• When you create a new note: after you create the collection, you'd expect to be redirected to the previous screen
• Later: when you import notes: same behavior as the above

[tasn]

Maybe I'm misunderstanding, but:

* From the drawer: after you create the collection, you'd expect to be redirected to the newly created collection

OK, so after a create there, just redirect.

* When you create a new note: after you create the collection, you'd expect to be redirected to the previous screen

After a create there, just redirect to the right place.

* Later: when you import notes: same behavior as the above

Same.

Maybe I'm just misunderstanding the patch, but I don't understand why the behaviour needs to be centralised. Just do whatever you need to do in whatever view you are. Or are your examples about changing the history stack because you want the back button to behave a certain way? I still don't understand what you are trying to achieve.

[zecakeh]

This patch is to avoid to hardcode the screens in CollectionEditScreen, but rather let the screen that initiated the new notebook tell CollectionEditScreen to send back the colUid. That allows us to add views with that behavior without having to touch CollectionEditScreen. I don't see it as more complex, but maybe you do.

If you're fine with hardcoding the screens, this PR should be closed.

[tasn]

Definitely not fine with hardcoding. I just don't understand what this patch really does it seems. Could you maybe describe the purpose of this patch in plain words again? As I still don't get it.

[zecakeh]

Okay then maybe you're seeing a solution that I don't. I'll try to explain in detail:

Use case 1, the user creates a Notebook from the drawer:

1. The user can be on any screen (because the Menu button can be available in any screen on web).
2. Press Create new notebook in the drawer
3. The drawer triggers navigation to CollectionEditScreen without params
4. The user fills the form and presses the Save button
5. CollectionEditScreen stores the info, and since it didn't receive params, triggers navigation forward or back to NotesListScreen with the new colUid as a param
6. The user is on the newly created Notebook's screen

Use case 2, the user creates a Note, and realizes it should be in a new Notebook:

1. The user is in NotePropertiesScreen
2. Press the + button in the Notebook select
3. The NotePropertiesScreen triggers navigation to CollectionEditScreen without param goBack: true
4. The user fills the form and presses the Save button
5. CollectionEditScreen stores the info, and since it received the param goBack, triggers navigation back to NotePropertiesScreen with the new colUid as a param
6. The user is back on the New Note screen, with the new notebook selected

[tasn]

OK, I think this is where the confusion stemmed from. I didn't remember that the menu button is always visible on the web... Is this change still needed now that we have different tabs? Or are the different tabs just for the home screen?

And sorry for maybe asking the same question twice but why not always go back? Because sometimes it's from a screen that you don't want to go to, and in that case you want to just go to a certain page? Anyhow, I'm fine with this change if this is the case, just make sure to fix the conflicts and let me know.

[zecakeh]

Previously it's because you could open "Create Notebook" from any page on web (i.e. the Settings if you opened directly notes.etesync.com/settings) so it was more logical to navigate to the Notebook's page. However it's not in the Drawer anymore, so I guess we can always go back… I'll check that and change it accordingly.

[tasn]

Let's always go back then, I think that's better. :) I try to avoid complexity as much as possible, and every condition or parameter are complexity.

[zecakeh]

Changed and ready for review

Edit: I just realized this won't work with split view because you'll be able to create a new notebook on every screen, since the list is always present on the left. So I'd say just use goBack and store the open notebook in state to be able to change it from CollectionCreate

[sturdy-devtools[bot]]

Potential conflicts warning

This pull request has changes that are conflicting with the changes in 2 open PRs.

#163 Rename RootStackParamList.tsx to NavigationConfig.tsx by @zecakeh (updated 5 minutes ago)

• src/RootStackParamList.tsx

#164 Move linking config to NavigationConfig by @zecakeh (updated Just now)

• src/RootStackParamList.tsx

[zecakeh]

So I made the modifications I suggested. We now lose the feature that selects the newly created Notebook in the New Note form. To do that I do believe I need to reintroduce a route param.