RNTester: Update Image page and remove assert #4842
Conversation
| // #4691 | ||
| assert(value != ResizeMode::Repeat); | ||
|
|
||
| default: // ResizeMode::Center || ResizeMode::Repeat |
There was a problem hiding this comment.
consider replacing default: with:
case ResizeMode::Center:
case ResizeMode::Repeat:
<code goes here>
default:
assert("this should not be hit");
}
| @@ -26,7 +26,7 @@ const base64Icon = | |||
|
|
|||
| const ImageCapInsetsExample = require('./ImageCapInsetsExample'); | |||
| const IMAGE_PREFETCH_URL = | |||
| 'http://origami.design/public/images/bird-logo.png?r=1&t=' + Date.now(); | |||
| 'https://www.facebook.com/favicon.ico?r=1&t=' + Date.now(); | |||
There was a problem hiding this comment.
ReactCopies needs to remain an exact copy of upstream RN at the current version, and will get overwritten during updates.
There was a problem hiding this comment.
What version was the change in? If we need to add a new version of this to our fork we can, but we will likely need to do it a different way.
There was a problem hiding this comment.
Looks like we'll get this with 63. Didn't look to see if it all came into nightly bits.
Could we leave the assertion removed but leave tester as is? I'm going to start integrating a 63-ish release next week. Bad links should recoverably redbox, right?
There was a problem hiding this comment.
If this needs to match facebooks, repo, should we at least send a PR to facebook to declare this url as a private const in the file?
It seems silly to repeat a URL many times ;)
There was a problem hiding this comment.
I mean that this exact file is a copy from React Native at a given version. There's an ask for them to publish source for this as a package, but right now we have tooling to ensure it stays up to date.
There was a problem hiding this comment.
I can revert the RNTester changes, but I only updated it because one of the urls they changed is now dead: https://facebook.github.io/react-native/img/favicon.png
There was a problem hiding this comment.
Can definitely understand the intent here, but forking is expensive in general, and we should avoid it where possible. Can explain more about that if interested.
Do the wrong URLs prevent validating the change? That's a case where we might be okay making changes, but we need to do it in a slightly different way.
But in general, my preference would be to wait until we get Facebook's, especially since it's coming in the version we're about to upgrade to.
ghost
added
the
Needs: Author Feedback
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
removed
the
Needs: Author Feedback
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
@marlenecota you might need to rebase to clear the verdaccio error: ERROR: Attempting to bump to a version that already exists in the registry: [email protected]
Validating package version - [email protected] package version validation - package e2etest is private
Something went wrong with the publish! Manually update these package and versions:
Skipping package version validation - package microsoft-reactnative-sampleapps is private
- [email protected] |
|
@marlenecota "is this still available"? :) |
marlenecota
added
the
AutoMerge
|
Hello @marlenecota! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
Fix for #4691
Microsoft Reviewers: Open in CodeFlow