Refactor Calligraphy into a ViewPump interceptor #1
Conversation
chrisjenx
left a comment
chrisjenx
left a comment
There was a problem hiding this comment.
Let's also make this 14+ not that it probably makes much difference?
| } | ||
|
|
||
| public Builder setFontMapper(FontMapper fontMapper) { | ||
| this.fontMapper = fontMapper; |
There was a problem hiding this comment.
Can we move this FontMapper into a different PR?
|
|
||
| import io.github.inflationx.viewpump.ReflectionUtils; | ||
|
|
||
| class CalligraphyFactory { |
There was a problem hiding this comment.
Do you think this should still be called Factory? The ViewPump#Interceptor calls into these methods right?
There was a problem hiding this comment.
Factory doesn't make as much sense now since it's further from the LayoutInflater.Factory, but I can't think of a better name. maybe just Calligraphy?
| textViewFont = CalligraphyUtils.pullFontPathFromTheme(context, styleForTextView[0], mAttributeId); | ||
| } | ||
|
|
||
| textViewFont = applyFontMapper(textViewFont); |
There was a problem hiding this comment.
again pull out into a seperate PR
|
|
||
| dependencies { | ||
| compile 'io.github.inflationx:viewpump:0.1.0-SNAPSHOT' | ||
| compile 'com.android.support:appcompat-v7:24.0.0' |
There was a problem hiding this comment.
@chrisjenx what do you think about dependencies here? keep viewpump compile or change to provided? what about appcompat, too?
sounds good. I just kept it the same as the original lib, but happy to make the bump to 14 |
|
Addressed comments |
With the LayoutInflater and ContextWrapper moved to ViewPump, Calligraphy can be refactored into a ViewPump interceptor.
This refactored version of Calligraphy will be namespaced as
calligraphy3due to the major breaking changes. This PR depends on SNAPSHOT version of ViewPump provided throughmavenLocal(), but will update it when a hosted snapshot becomes available.