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

Customize tile layout #165

Merged
merged 2 commits into from Aug 29, 2017
Merged

Customize tile layout #165

merged 2 commits into from Aug 29, 2017

Conversation

dlackty
Copy link
Contributor

@dlackty dlackty commented Nov 3, 2016

For some use cases, library user might want to swap the layout for image grid item.

@dlackty dlackty changed the base branch from master to z/transformers_and_API December 2, 2016 07:12
@dlackty dlackty changed the base branch from z/transformers_and_API to master December 2, 2016 07:12
@dlackty
Copy link
Contributor Author

dlackty commented Dec 2, 2016

@Taresin Just rebased the PR on latest master. Could you please help review the changes? Also I would like to sign the CLA but the link on README leads to 404.

@dlackty
Copy link
Contributor Author

dlackty commented Aug 23, 2017

@markrietveld Hello, any chance to get this reviewed and merged in? I thought it's quite trivial change and would make this awesome library even more flexible.

@markrietveld
Copy link
Contributor

@dlackty sorry for the silence leading up to this. I missed this pull request.

What is the use case for this change? The inflated layout needs to be an ImageView, or at least extend ImageView. That doesn't provide a lot of flexibility. Depending on what you're trying to achieve, there might be a better way to do it.

@dlackty
Copy link
Contributor Author

dlackty commented Aug 23, 2017

Thanks for the response! What we want to achieve is to use BottomSheet with Facebook's Fresco library. i.e. Replace the default tile with SimpleDraweeView, which is an ImageView subclass.

@@ -501,6 +509,18 @@ public Builder setPickerDrawable(@DrawableRes int resId) {
}

/**
* Sets a layout for the image tile. Default is to use plain image view included in the
* library.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add that the root of the layout must be an ImageView, or the sheet will crash at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@markrietveld Resolved and also added a type check.

@markrietveld
Copy link
Contributor

@dlackty thanks for contributing!

@markrietveld markrietveld merged commit 43ab062 into Flipboard:master Aug 29, 2017
@dlackty
Copy link
Contributor Author

dlackty commented Aug 30, 2017

Thanks @markrietveld!

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.

None yet

2 participants