Skip to content

[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

Merged
merged 1 commit into from
Aug 10, 2015

Conversation

mdziekon
Copy link
Contributor

@mdziekon mdziekon commented Aug 8, 2015

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.)

@AlexKVal
Copy link
Member

AlexKVal commented Aug 8, 2015

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"

here

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.

@mdziekon
Copy link
Contributor Author

mdziekon commented Aug 8, 2015

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.

@AlexKVal
Copy link
Member

AlexKVal commented Aug 9, 2015

Ah.. Did think about it, sorry 😊

@dozoisch
Copy link
Member

dozoisch commented Aug 9, 2015

LGTM

1 similar comment
@mtscout6
Copy link
Member

LGTM

mtscout6 added a commit that referenced this pull request Aug 10, 2015
[added] Thumbnail links support
@mtscout6 mtscout6 merged commit 055f143 into react-bootstrap:master Aug 10, 2015
@taion
Copy link
Member

taion commented Aug 10, 2015

I wonder if we can use some wrapper component for these generic linked components - i.e. just pass down getLinkProps to the wrapped component.

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

Successfully merging this pull request may close these issues.

5 participants