-
Notifications
You must be signed in to change notification settings - Fork 163
[added] Thumbnail links support #102
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
Conversation
assets folder is OK (R-B does the same)
LGTM ❇️
Though I'm not sure about:
This PR changes the minimum version of R-B dependency:
from 0.15 to
"react-bootstrap": ">=0.22.4"
Anyway it needs one more approval of @react-bootstrap/collaborators
If this PR merged as is, then the next version of R-R-B will be 0.19 because of this "react-bootstrap": ">=0.22.4" change, I presume.
According to R-B's Changelog, Thumbnail component was introduced in version 0.22.4 (https://github.com/react-bootstrap/react-bootstrap/blob/master/CHANGELOG.md#v0224---mon-18-may-2015-165306-gmt)
I guessed dep version had to be changed to make sure we have this component in library.
Ah.. Did think about it, sorry 😊
LGTM
1 similar comment
LGTM
[added] Thumbnail links support
I wonder if we can use some wrapper component for these generic linked components - i.e. just pass down getLinkProps to the wrapped component.
This PR implements links support for
<Thumbnail>component as<ThumbnailLink>.This is how visual test looks like:
react-router-bootstrap-thumbnail-visual
(note: as you can see, I've used Holder.js generated placeholder for the thumbnail, but I'm not sure whether "/assets" dir is a good place for it or not. If anyone have a better idea, please let me know.)