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

Major fixes #59

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions CachedImage.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ const ImageCacheProvider = require('./ImageCacheProvider');

const {
View,
Image,
ImageBackground,
ActivityIndicator,
NetInfo,
Platform
Platform,
Text,
} = ReactNative;


Expand All @@ -34,7 +35,7 @@ const styles = StyleSheet.create({
});

function getImageProps(props) {
return _.omit(props, ['source', 'defaultSource', 'fallbackSource', 'LoadingIndicator', 'activityIndicatorProps', 'style', 'useQueryParamsInCacheKey', 'renderImage', 'resolveHeaders']);
return _.omit(props, ['source', 'defaultSource', 'fallbackSource', 'LoadingIndicator', 'activityIndicatorProps', 'style', 'useQueryParamsInCacheKey', 'renderImage']);
}

const CACHED_IMAGE_REF = 'cachedImage';
Expand All @@ -47,15 +48,13 @@ const CachedImage = React.createClass({
React.PropTypes.bool,
React.PropTypes.array
]).isRequired,
resolveHeaders: React.PropTypes.func
},

getDefaultProps() {
return {
renderImage: props => (<Image ref={CACHED_IMAGE_REF} {...props}/>),
renderImage: props => (<ImageBackground ref={CACHED_IMAGE_REF} {...props}/>),
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you prefer ImageBackground over Image?

Copy link
Author

Choose a reason for hiding this comment

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

React recommends to use ImageBackground.

Copy link
Contributor

Choose a reason for hiding this comment

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

To confirm, using ImageBackground would require implementors of react-native-cached-image to update to 0.46? If so and @kfiroo is happy with this, this should probably be denoted as a peerDependency in the package.json.

Copy link
Author

Choose a reason for hiding this comment

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

Added alternative in "progressive enhancement" style for ImageBackground facebook/react-native@9637dd4

activityIndicatorProps: {},
useQueryParamsInCacheKey: false,
resolveHeaders: () => Promise.resolve({})
};
},

Expand All @@ -72,7 +71,8 @@ const CachedImage = React.createClass({
return {
isCacheable: false,
cachedImagePath: null,
networkAvailable: true
networkAvailable: true,
errorLoading: false,
};
},

Expand Down Expand Up @@ -117,20 +117,25 @@ const CachedImage = React.createClass({
processSource(source) {
const url = _.get(source, ['uri'], null);
if (ImageCacheProvider.isCacheable(url)) {
const options = _.pick(this.props, ['useQueryParamsInCacheKey', 'cacheGroup']);
const options = _.pick(this.props, ['useQueryParamsInCacheKey', 'cacheGroup', 'source']);
// try to get the image path from cache
ImageCacheProvider.getCachedImagePath(url, options)
.then(ImageCacheProvider.isExpired)
// try to put the image in cache if
.catch(() => ImageCacheProvider.cacheImage(url, options, this.props.resolveHeaders))
.catch(() => ImageCacheProvider.cacheImage(url, options))
.then(cachedImagePath => {
if(!cachedImagePath) {
throw new Error('Failed to cache image')
}
this.safeSetState({
cachedImagePath
});
})
.catch(err => {
this.safeSetState({
cachedImagePath: null,
isCacheable: false
isCacheable: false,
errorLoading: true,
});
});
this.safeSetState({
Expand All @@ -147,6 +152,9 @@ const CachedImage = React.createClass({
if (this.state.isCacheable && !this.state.cachedImagePath) {
return this.renderLoader();
}
if( this.state.errorLoading ) {
return this.renderError();
}
const props = getImageProps(this.props);
const style = this.props.style || styles.image;
const source = (this.state.isCacheable && this.state.cachedImagePath) ? {
Expand Down Expand Up @@ -211,6 +219,16 @@ const CachedImage = React.createClass({
style={activityIndicatorStyle}/>
)
});
},

renderError() {
Copy link
Owner

Choose a reason for hiding this comment

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

We would probably want to give the users a way to render their own errors.
What do you think? A prop for renderError?

Copy link
Author

Choose a reason for hiding this comment

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

Good.

const imageStyle = [this.props.style, styles.loaderPlaceholder];
const activityIndicatorStyle = this.props.activityIndicatorProps.style || styles.loader;
return (
<View style={[imageStyle, activityIndicatorStyle]}>
<Text>Error</Text>
</View>
);
}
});

Expand Down
4 changes: 2 additions & 2 deletions CachedImageExample/.babelrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"presets": ["react-native"]
}
"presets": ["react-native"]
}
11 changes: 6 additions & 5 deletions CachedImageExample/.flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ node_modules/react-native/flow
flow/

[options]
module.system=haste
emoji=true

experimental.strict_type_args=true
module.system=haste

munge_underscores=true

Expand All @@ -35,11 +35,12 @@ suppress_type=$FlowIssue
suppress_type=$FlowFixMe
suppress_type=$FixMe

suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(3[0-5]\\|[1-2][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)
suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(3[0-5]\\|1[0-9]\\|[1-2][0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)?:? #[0-9]+
suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(4[0-7]\\|[1-3][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)
suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(4[0-7]\\|[1-3][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)?:? #[0-9]+
suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy
suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError

unsafe.enable_getters_and_setters=true

[version]
^0.35.0
^0.47.0
2 changes: 1 addition & 1 deletion CachedImageExample/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ local.properties
#
node_modules/
npm-debug.log
yarn-error.log

# BUCK
buck-out/
\.buckd/
android/app/libs
*.keystore
53 changes: 26 additions & 27 deletions CachedImageExample/android/app/BUCK
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import re

# To learn about Buck see [Docs](https://buckbuild.com/).
# To run your application with Buck:
# - install Buck
Expand All @@ -11,56 +9,57 @@ import re
#

lib_deps = []

for jarfile in glob(['libs/*.jar']):
name = 'jars__' + re.sub(r'^.*/([^/]+)\.jar$', r'\1', jarfile)
name = 'jars__' + jarfile[jarfile.rindex('/') + 1: jarfile.rindex('.jar')]
lib_deps.append(':' + name)
prebuilt_jar(
name = name,
binary_jar = jarfile,
)

for aarfile in glob(['libs/*.aar']):
name = 'aars__' + re.sub(r'^.*/([^/]+)\.aar$', r'\1', aarfile)
name = 'aars__' + aarfile[aarfile.rindex('/') + 1: aarfile.rindex('.aar')]
lib_deps.append(':' + name)
android_prebuilt_aar(
name = name,
aar = aarfile,
)

android_library(
name = 'all-libs',
exported_deps = lib_deps
name = "all-libs",
exported_deps = lib_deps,
)

android_library(
name = 'app-code',
srcs = glob([
'src/main/java/**/*.java',
]),
deps = [
':all-libs',
':build_config',
':res',
],
name = "app-code",
srcs = glob([
"src/main/java/**/*.java",
]),
deps = [
":all-libs",
":build_config",
":res",
],
)

android_build_config(
name = 'build_config',
package = 'com.cachedimageexample',
name = "build_config",
package = "com.cachedimageexample",
)

android_resource(
name = 'res',
res = 'src/main/res',
package = 'com.cachedimageexample',
name = "res",
package = "com.cachedimageexample",
res = "src/main/res",
)

android_binary(
name = 'app',
package_type = 'debug',
manifest = 'src/main/AndroidManifest.xml',
keystore = '//android/keystores:debug',
deps = [
':app-code',
],
name = "app",
keystore = "//android/keystores:debug",
manifest = "src/main/AndroidManifest.xml",
package_type = "debug",
deps = [
":app-code",
],
)
9 changes: 8 additions & 1 deletion CachedImageExample/android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ import com.android.build.OutputFile
* // bundleInPaidRelease: true,
* // bundleInBeta: true,
*
* // whether to disable dev mode in custom build variants (by default only disabled in release)
* // for example: to disable dev mode in the staging build type (if configured)
* devDisabledInStaging: true,
* // The configuration property can be in the following formats
* // 'devDisabledIn${productFlavor}${buildType}'
* // 'devDisabledIn${buildType}'
*
* // the root of your project, i.e. where "package.json" lives
* root: "../../",
*
Expand All @@ -58,7 +65,7 @@ import com.android.build.OutputFile
* inputExcludes: ["android/**", "ios/**"],
*
* // override which node gets called and with what additional arguments
* nodeExecutableAndArgs: ["node"]
* nodeExecutableAndArgs: ["node"],
*
* // supply additional arguments to the packager
* extraPackagerArgs: []
Expand Down
4 changes: 4 additions & 0 deletions CachedImageExample/android/app/proguard-rules.pro
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@

-dontwarn com.facebook.react.**

# TextLayoutBuilder uses a non-public Android constructor within StaticLayout.
# See libs/proxy/src/main/java/com/facebook/fbui/textlayoutbuilder/proxy for details.
-dontwarn android.text.StaticLayout

# okhttp

-keepattributes Signature
Expand Down
4 changes: 3 additions & 1 deletion CachedImageExample/android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
<activity
android:name=".MainActivity"
android:label="@string/app_name"
android:configChanges="keyboard|keyboardHidden|orientation|screenSize">
android:configChanges="keyboard|keyboardHidden|orientation|screenSize"
android:windowSoftInputMode="adjustResize"
>
<intent-filter>
<action android:name="android.intent.action.MAIN"/>
<category android:name="android.intent.category.LAUNCHER"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class MainApplication extends Application implements ReactApplication {

private final ReactNativeHost mReactNativeHost = new ReactNativeHost(this) {
@Override
protected boolean getUseDeveloperSupport() {
public boolean getUseDeveloperSupport() {
return BuildConfig.DEBUG;
}

Expand Down
2 changes: 1 addition & 1 deletion CachedImageExample/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ buildscript {
jcenter()
}
dependencies {
classpath 'com.android.tools.build:gradle:1.3.1'
classpath 'com.android.tools.build:gradle:2.2.3'

// NOTE: Do not place your application dependencies here; they belong
// in the individual module build.gradle files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-2.4-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-2.14.1-all.zip
12 changes: 6 additions & 6 deletions CachedImageExample/android/keystores/BUCK
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
keystore(
name = 'debug',
store = 'debug.keystore',
properties = 'debug.keystore.properties',
visibility = [
'PUBLIC',
],
name = "debug",
properties = "debug.keystore.properties",
store = "debug.keystore",
visibility = [
"PUBLIC",
],
)
4 changes: 4 additions & 0 deletions CachedImageExample/app.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "CachedImageExample",
"displayName": "CachedImageExample"
}
Loading