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

[#1579] update ImageBackground component doc #1598

Open
wants to merge 3 commits into
base: master
from

Conversation

@alvessteve
Copy link

alvessteve commented Feb 5, 2020

Here is an update for the #1579 regarding the ImageBackground component.

The example was either a class or a function component so I turned it into one functional.

@react-native-bot

This comment has been minimized.

Copy link

react-native-bot commented Feb 5, 2020

Deploy preview for react-native ready!

Built with commit 100a80c

https://deploy-preview-1598--react-native.netlify.com

Changes to docs/ are reflected in the next "master" version.

Thank you for your contributions.

How to ContributeDocumentation Sources

@alvessteve alvessteve requested a review from rachelnabors Feb 6, 2020

```SnackPlayer name=ImageBackground
import React from 'react';
import {View, ImageBackground, Text} from 'react-native'

This comment has been minimized.

Copy link
@Simek

Simek Feb 7, 2020

Contributor

Imports alphabetic order + missing semicolon.

import React from 'react';
import {View, ImageBackground, Text} from 'react-native'
export default function App() {

This comment has been minimized.

Copy link
@Simek

Simek Feb 7, 2020

Contributor

Arrow function.

return (
<View>
<ImageBackground source={{uri: 'https://facebook.github.io/react/logo-og.png'}} style={{width: '100%', height: '100%'}}>
<Text style ={{color: 'white', fontSize: 20, alignSelf: 'center'}}>Inside</Text>

This comment has been minimized.

Copy link
@Simek

Simek Feb 7, 2020

Contributor

Unnecessary space after style.

</ImageBackground>
</View>
)
}

This comment has been minimized.

Copy link
@Simek

Simek Feb 7, 2020

Contributor

Missing semicolon.

export default function App() {
return (
<View>
<ImageBackground source={{uri: 'https://facebook.github.io/react/logo-og.png'}} style={{width: '100%', height: '100%'}}>

This comment has been minimized.

Copy link
@Simek

Simek Feb 7, 2020

Contributor

I would place source and style in separate lines (like for example TouchableNativeFeedback) since Snack code editor is quite narrow.

@lcelso

This comment has been minimized.

Copy link
Contributor

lcelso commented Feb 7, 2020

Sugestion:

import React from 'react';
import { ImageBackground, StyleSheet, Text, View } from 'react-native';

const image = 'https://bit.ly/reactBR';

export default App = () => (
  <View style={styles.container}>
    <ImageBackground 
    source={image} style={styles.image}>
      <Text style={styles.text}>Inside</Text>
    </ImageBackground>
  </View>
);

const styles = StyleSheet.create({
  container: {
    flex: 1,
    flexDirection: 'column',
  },
  image: {
    flex: 1,
    resizeMode: 'cover', // or 'stretch',
    justifyContent: 'center',
  },
  text: {
    fontSize: 30,
    fontWeight: 'bold',
  }
})
@Simek

This comment has been minimized.

Copy link
Contributor

Simek commented Feb 7, 2020

@lcelso Thank you for your suggestion.

I would change two more things, since you have extracted styles and URL to consts line breaking is no longer required:

export default App = () => (
  <View style={styles.container}>
    <ImageBackground source={image} style={styles.image}>
      <Text style={styles.text}>Inside</Text>
    </ImageBackground>
  </View>
);

and I would leave URL in original format, not shortened (you have missed uri):

const image = { uri: 'https://facebook.github.io/react/logo-og.png' };

Also semicolon is missing after styles const but other than that LGTM 😃

@lcelso

This comment has been minimized.

Copy link
Contributor

lcelso commented Feb 7, 2020

@Simek I have a branch for this if you want I can go up

@Simek

This comment has been minimized.

Copy link
Contributor

Simek commented Feb 7, 2020

@lcelso I'm not here to decide, we should wait until one of maintainers take part in the discussion.

Copy link
Contributor

rachelnabors left a comment

I'd like to see this with @Simek's suggestions in place please :)

@alvessteve

This comment has been minimized.

Copy link
Author

alvessteve commented Feb 20, 2020

Sorry for the delay. Thank you for the suggestions ! I will add @Simek suggestions then

Steve Alves-Blyt and others added 2 commits Feb 5, 2020
@alvessteve alvessteve force-pushed the alvessteve:1579-imagebackground branch from 32b3711 to 156ec03 Feb 21, 2020
@alvessteve

This comment has been minimized.

Copy link
Author

alvessteve commented Feb 21, 2020

changes done. @Simek, had to force push because of confused git commands. Don't know the side effects, if you want i can close this branch and create another one.

Prettier-ed
Copy link
Contributor

rachelnabors left a comment

Thanks so much for adding this example! You're helping the docs be more and more useful for thousands of learners every day!
mlp_rarity-excite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.