Added the ability to set an image title in the NavigatorIOS #8182
Conversation
|
By analyzing the blame information on this pull request, we identified @ericvicenti and @janicduplessis to be potential reviewers. |
ghost
added
GH Review: review-needed
CLA Signed
| title: 'Custom title image Example', | ||
| titleImage: require('./relay.png'), | ||
| component: createExamplePage(null, ViewExample), | ||
| }) |
alvaromb
force-pushed
the
navigator_img
branch
2 times, most recently
from
64e3656 to
9e3ebbe
Compare
| - (UIImageView *)titleImageView | ||
| { | ||
| if (!_titleImageView) { | ||
| if (_titleImage) { |
There was a problem hiding this comment.
This will not be properly invalidated when you update the titleImage property.
There was a problem hiding this comment.
Thanks for jumping in @javache!
Yes, you're right, I reproduced the issue using replacePreviousAndPop with a previous route with an image and it doesn't get updated. I'll update the PR.
| - (UIImageView *)titleImageView | ||
| { | ||
| if (_titleImage) { | ||
| _titleImageView = [[UIImageView alloc] initWithImage:_titleImage]; |
There was a problem hiding this comment.
Since we're always recreating it, it doesn't seem to be useful to keep it around as an instance variable. Could you remove the @synthesize and just return the UIImageView immediately?
I'm assuming this will be efficient enough since it only gets called on viewWillAppear. If not, you need to figure out a way to reuse the _titleImageView here if the image didn't change.
There was a problem hiding this comment.
I'm in favour of removing the @synthesize, comparing UIImages will be more expensive. Will update the PR later today.
|
There you go @javache! AFAIK, removing |
|
@facebook-github-bot shipit |
ghost
added
GH Review: accepted
Import Started
|
Thanks for importing. If you are an FB employee go to Phabricator to review. |
|
I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification. |
ghost
added
Import Failed
and removed
Import Started
Added an example to UIExplorer Solved lint issue Added the ability to set an image title in the NavigatorIOS Added an example to UIExplorer Solved lint issue Added the ability to set an image title in the NavigatorIOS Added an example to UIExplorer Solved lint issue Properly regenerated the title image view Removed @synthesize to avoid ivar creation for the titleImageView and generated the UIImageView on the fly
Summary: This pull request adds support for setting an image title in `NavigatorIOS`. It sets the `titleView` property of `UINavigationItem`. Added an example into `UIExplorer` <img width="487" alt="ex" src="https://cloud.githubusercontent.com/assets/289640/16147578/3d552606-3484-11e6-9a26-1e8c113ace03.png"> The code follows the same style as the rest of `RCTNavItem` and `RCTWrapperViewController`. Closes facebook/react-native#8182 Differential Revision: D3469073 Pulled By: javache fbshipit-source-id: 2d3e4bad534f92b6e6b24a6b74f2993f3a8c9012
Summary: This pull request adds support for setting an image title in `NavigatorIOS`. It sets the `titleView` property of `UINavigationItem`. Added an example into `UIExplorer` <img width="487" alt="ex" src="https://cloud.githubusercontent.com/assets/289640/16147578/3d552606-3484-11e6-9a26-1e8c113ace03.png"> The code follows the same style as the rest of `RCTNavItem` and `RCTWrapperViewController`. Closes facebook#8182 Differential Revision: D3469073 Pulled By: javache fbshipit-source-id: 2d3e4bad534f92b6e6b24a6b74f2993f3a8c9012
Summary: This pull request adds support for setting an image title in `NavigatorIOS`. It sets the `titleView` property of `UINavigationItem`. Added an example into `UIExplorer` <img width="487" alt="ex" src="https://cloud.githubusercontent.com/assets/289640/16147578/3d552606-3484-11e6-9a26-1e8c113ace03.png"> The code follows the same style as the rest of `RCTNavItem` and `RCTWrapperViewController`. Closes facebook#8182 Differential Revision: D3469073 Pulled By: javache fbshipit-source-id: 2d3e4bad534f92b6e6b24a6b74f2993f3a8c9012
I made sure 1.0 is backwards compatible with react-navigation, which means using rn-safe-area-context@1+ with older versions of react-navigation will still work.
Motivation
This pull request adds support for setting an image title in
NavigatorIOS. It sets thetitleViewproperty ofUINavigationItem.Example
Added an example into
UIExplorer
## Coding styleThe code follows the same style as the rest of
RCTNavItemandRCTWrapperViewController.