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

Add information about styling ListItems #1915

Closed
wants to merge 1 commit into from

Conversation

lukewlms
Copy link
Contributor

@lukewlms lukewlms commented Jun 6, 2019

This seems like an important change to me because the writing here doesn't match the images. Adding the example code will not produce the pictured results, because there is no styling in the example code. The code in this comment does produce nice results.

See one frustrated user here: https://github.com/react-native-training/react-native-elements/issues/1565#issuecomment-499344338 - we also hit this issue and spent probably a couple hours trying to figure out why styling had died after an upgrade.

I believe this issue will be a significant hurdle for new users, and hurts the initial experience quite a lot. It's just not possible to use ListItems without styling (they will look terrible) and this is a library that usually provides at least reasonable styling right away, so at minimum an example seems called for. This PR is one way to do it, ofc more docs could be written as well.

Thanks for considering - we use this lib heavily and want it to be a success!

This seems like an important change to me because the writing here doesn't match the images.  Adding the example code will _not_ produce the pictured results, because there is no styling in the example code.  The code in this comment does produce nice results.

See one frustrated user here: https://github.com/react-native-training/react-native-elements/issues/1565#issuecomment-499344338 - we also hit this issue and spent probably a couple hours trying to figure out why styling had died after an upgrade.

I believe this issue will be a significant hurdle for new users, and hurts the initial experience quite a lot.   It's just not possible to use ListItems without styling (they will look terrible) and this is a library that usually provides at least reasonable styling right away, so at minimum an example seems called for.  This PR is one way to do it, ofc more docs could be written as well.

Thanks for considering - we use this lib heavily and want it to be a success!
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #1915 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1915   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files          33       33           
  Lines         573      573           
  Branches       73       73           
=======================================
  Hits          544      544           
  Misses         25       25           
  Partials        4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c900a4...01a7e27. Read the comment docs.

@iRoachie
Copy link
Collaborator

iRoachie commented Jul 6, 2019

The issue you linked was referencing upgrading from 0.19.1 to v1.0.0 of the library, in which the styles the author previously added on the list item didn't have the same effect in v1.0.0.

The link to the comment you want to add to the docs, will not give the same styling as the images. The images only show what's possible with customization.

I think we can add a different message and maybe put it under the Image on the ListItem page. https://react-native-training.github.io/react-native-elements/docs/listitem.html

What do you think?

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

Successfully merging this pull request may close these issues.

None yet

2 participants