Skip to content

Add unregisterForPreviewingWithContext#2239

Merged
guyca merged 26 commits into
wix:masterfrom
birkir:feature/preview-peek-pop
Dec 13, 2017
Merged

Add unregisterForPreviewingWithContext#2239
guyca merged 26 commits into
wix:masterfrom
birkir:feature/preview-peek-pop

Conversation

@birkir
Copy link
Copy Markdown
Contributor

@birkir birkir commented Nov 28, 2017

  • Keep the preview controller context.
  • Unregister for previewing on next screen push that is a preview.

3D Touch diagram

Sometimes when previewing different screens, they would "cache" and preview the correct one, but commit the old one.

This seems to fix that problem, also it is just a good idea to keep the context. It could be used later to allow users to unregister via an API.

@birkir birkir mentioned this pull request Nov 28, 2017
8 tasks
Copy link
Copy Markdown

@enahum enahum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome @birkir thank you, can't wait for this to go in

@rgoldiez
Copy link
Copy Markdown
Contributor

@birkir - I applied this patch manually and I'm still seeing the same issue on previews. Seems to happen when trying to preview the same component but with different props (and from the same screen). In my case, I have a conversation feed and the preview functionality is to peak at a user's profile (or a tap / force'd press to go to their profile). So on the same base screen you can preview the same component (userProfile) with different props for different users.

@enahum
Copy link
Copy Markdown

enahum commented Nov 29, 2017

@rgoldiez I have a screen that receives props and is being shown with peek, that same screen shows with different props for a list of different items in your case userProfile in my case Channels and it works perfectly

this is the code for our app if you want to take a look mattermost/mattermost-mobile#1203

but without this PR it does show the last cached screen

@enahum
Copy link
Copy Markdown

enahum commented Nov 29, 2017

@birkir I think this branch needs another rebase

@birkir
Copy link
Copy Markdown
Contributor Author

birkir commented Nov 29, 2017

Done. Also after looking at your PR for mattermost, I see that you are not committing the controller because it is just updating the UI rather than pushing a new screen.

What we could do is dispatch an event to the topViewController saying that the preview was deep pressed, so you could add an event listener and call onSelectChannel.

Just a thought to extend this even further.

@enahum
Copy link
Copy Markdown

enahum commented Nov 29, 2017

Yeah that might be pretty neat

@guyca
Copy link
Copy Markdown
Collaborator

guyca commented Nov 30, 2017

@birkir What's the status of this PR? is it ready or requires some more revisions?

@birkir
Copy link
Copy Markdown
Contributor Author

birkir commented Dec 1, 2017

This is ready to be merged!

@enahum
Copy link
Copy Markdown

enahum commented Dec 4, 2017

is there anything else needed for this PR to get merged?

@birkir
Copy link
Copy Markdown
Contributor Author

birkir commented Dec 6, 2017

Just waiting for a moderator to review and merge.

@enahum
Copy link
Copy Markdown

enahum commented Dec 7, 2017

friendly ping to @guyca ;)

@guyca guyca merged commit 36ed3b0 into wix:master Dec 13, 2017
@birkir birkir deleted the feature/preview-peek-pop branch December 13, 2017 15:46
chilinh added a commit to chilinh/react-native-navigation that referenced this pull request Dec 18, 2017
* r_master:
  fixed toggle tabs bug (wix#2369)
  Add paddingTop to android ToolBar. (wix#2266)
  [Android] TopTabs font family (wix#2342)
  Ensure correct spelling of commandType (wix#2246)
  Add unregisterForPreviewingWithContext (wix#2239)
  Fix rn 0.50 repeated calls to onMeasure. (wix#2261)
  [Android] Support for transparent statusbar with current screen rendered behind it (wix#2274)
  Fix NPE when activity is relaunched on changing font size of the phone (wix#2319)
  Missed comma (wix#2305)
  Add commandType param to props which indicates if a screen is displayed using push or modal
  Find textual buttons in TitleBar by text instead of content description
  Add a toString implementation to StyleParams Color (wix#2313)
garrettm pushed a commit to Ginger-Labs/react-native-navigation that referenced this pull request Dec 19, 2017
* Preview API

* Added preview actions and fail-guard wrappers

* Remove old setters

* Moved from passProps to actionParams

* Move findNodeHandle to internal method

* Removing findNodeHandle

* Added height and option to dont 'pop' the view controller

* Documentation update for peek and pop

* Document how to access button press events

* Commit by default in example. Because it's cool

* Docs and if elses ifs then

* Unregister previous ViewController on attempt to preview.

* Merge conflict fail
garrettm pushed a commit to Ginger-Labs/react-native-navigation that referenced this pull request Dec 19, 2017
* Preview API

* Added preview actions and fail-guard wrappers

* Remove old setters

* Moved from passProps to actionParams

* Move findNodeHandle to internal method

* Removing findNodeHandle

* Added height and option to dont 'pop' the view controller

* Documentation update for peek and pop

* Document how to access button press events

* Commit by default in example. Because it's cool

* Docs and if elses ifs then

* Unregister previous ViewController on attempt to preview.

* Merge conflict fail
thanhzusu pushed a commit to thanhzusu/react-native-navigation that referenced this pull request Dec 21, 2017
* master:
  Now allowing the custom nav bar to take up the whole space on iOS (wix#2306)
  Fix formatting of code block in installation-ios (wix#2356)
  Added optional fixedWidth property to the Android side menu (wix#2380)
  await startApp
  on activity resumed (wix#2370)
  fixed toggle tabs bug (wix#2369)
  Add paddingTop to android ToolBar. (wix#2266)
  [Android] TopTabs font family (wix#2342)
  Ensure correct spelling of commandType (wix#2246)
  Add unregisterForPreviewingWithContext (wix#2239)
  Fix rn 0.50 repeated calls to onMeasure. (wix#2261)
  [Android] Support for transparent statusbar with current screen rendered behind it (wix#2274)
  Fix NPE when activity is relaunched on changing font size of the phone (wix#2319)
  Missed comma (wix#2305)
  Add commandType param to props which indicates if a screen is displayed using push or modal
  Find textual buttons in TitleBar by text instead of content description
  Add a toString implementation to StyleParams Color (wix#2313)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants