Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor hardcoded image paths out in RNTester/Image example, get rid of .ico file #44415

Closed
wants to merge 2 commits into from

Conversation

rshest
Copy link
Contributor

@rshest rshest commented May 5, 2024

Summary:

Changelog:

[Internal] -

RNTester Image example used hardcoded https://www.facebook.com/favicon.ico, which has an uncommon ICO format, for no good reason aside of just this image being served from facebook.com.

This diff:

  • Replaces the ICO image with a PNG one (which is still served from facebook.com
  • Factors out all the multiple hardcoded paths into constants, so that it's easier to make such changes in the future
  • Changes another image to something that is a bit better on the eyes when severely downscaled

Differential Revision: D56978929

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels May 5, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56978929

@analysis-bot
Copy link

analysis-bot commented May 5, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,495,289 +275
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,867,610 +39
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: a881b51
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56978929

rshest added a commit to rshest/react-native that referenced this pull request May 5, 2024
…f .ico file (facebook#44415)

Summary:
Pull Request resolved: facebook#44415

# Changelog:
[Internal] -

RNTester Image example used hardcoded `https://www.facebook.com/favicon.ico`, which has an uncommon ICO format, for no good reason aside of just this image being served from `facebook.com`.

This diff:
* Replaces the ICO image with a PNG one (which is still served from `facebook.com`
* Factors out all the multiple hardcoded paths into constants, so that it's easier to make such changes in the future
* Changes another image to something that is a bit better on the eyes when severely downscaled

Differential Revision: D56978929
@rshest rshest force-pushed the export-D56978929 branch from 05dddf6 to e14b776 Compare May 5, 2024 10:40
rshest added 2 commits May 5, 2024 08:17
…acebook#44414)

Summary:

# Changelog:
[Internal] - 

I noticed that "Image/Fade Duration" test in RNTester is practically useless, as at the moment one scrolls to the test, the fading is most probably had already ended.

This adds a "button" to refresh the image and be able to see the fading in again and again, if desired.

Differential Revision: D56978930
…f .ico file (facebook#44415)

Summary:

# Changelog:
[Internal] - 

RNTester Image example used hardcoded `https://www.facebook.com/favicon.ico`, which has an uncommon ICO format, for no good reason aside of just this image being served from `facebook.com`.

This diff:
* Replaces the ICO image with a PNG one (which is still served from `facebook.com`
* Factors out all the multiple hardcoded paths into constants, so that it's easier to make such changes in the future
* Changes another image to something that is a bit better on the eyes when severely downscaled

Differential Revision: D56978929
@rshest rshest force-pushed the export-D56978929 branch from e14b776 to 215054c Compare May 5, 2024 15:18
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56978929

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 5, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 362abb9.

kosmydel pushed a commit to kosmydel/react-native that referenced this pull request May 6, 2024
…f .ico file (facebook#44415)

Summary:
Pull Request resolved: facebook#44415

# Changelog:
[Internal] -

RNTester Image example used hardcoded `https://www.facebook.com/favicon.ico`, which has an uncommon ICO format, for no good reason aside of just this image being served from `facebook.com`.

This diff:
* Replaces the ICO image with a PNG one (which is still served from `facebook.com`
* Factors out all the multiple hardcoded paths into constants, so that it's easier to make such changes in the future
* Changes another image to something that is a bit better on the eyes when severely downscaled

Reviewed By: christophpurrer

Differential Revision: D56978929

fbshipit-source-id: c627d1671c8cb66e9a78f4382faa56e539b2f7b3
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
…f .ico file (facebook#44415)

Summary:
Pull Request resolved: facebook#44415

# Changelog:
[Internal] -

RNTester Image example used hardcoded `https://www.facebook.com/favicon.ico`, which has an uncommon ICO format, for no good reason aside of just this image being served from `facebook.com`.

This diff:
* Replaces the ICO image with a PNG one (which is still served from `facebook.com`
* Factors out all the multiple hardcoded paths into constants, so that it's easier to make such changes in the future
* Changes another image to something that is a bit better on the eyes when severely downscaled

Reviewed By: christophpurrer

Differential Revision: D56978929

fbshipit-source-id: c627d1671c8cb66e9a78f4382faa56e539b2f7b3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants