Font Family prop & RTL support for android #117
Conversation
There was a problem hiding this comment.
Thanks a lot for this change @alijamaliz. I left some general feedback, but overall happy with it.
According to and old comment #62 (comment) RTL was previously working; I never tested it myself, but for posterity can you please include a screenshot of the snackbar before and after this change?
I appreciate the good work!
README.md
Outdated
| @@ -65,6 +67,8 @@ Snackbar.show({ | |||
| | `action` | `object` (described below) | `undefined` (no button) | Optional config for the action button (described below). | | |||
| | `backgroundColor` | `string` or `style` | `undefined` (natively renders as black) | The background color for the whole Snackbar. | | |||
| | `color` | `string` or `style` | `undefined` (natively renders as white) | The text color for the title. | | |||
| | `fontFamily` | `string` | `undefined` (use native default font) | Font family of snackbar title and action (See **Font support for android** section below). | | |||
| | `RTL` | `boolean` | `false` | Direction of the snackbar (See **RTL support for android** section below). | | |||
There was a problem hiding this comment.
Personal convention preference: Please use lowercase rtl for the prop name (uppercase is still fine for text comments, of course)
There was a problem hiding this comment.
Can this be detected automatically? Seems like there should be some app-wide setting that determines whether it's RTL or not, rather than passing a prop 🤔
There was a problem hiding this comment.
I replaced all RTLs with rtl
There was a problem hiding this comment.
And about the second comment, it is a good idea.
I did little search and I found many approaches to determine that.
It can be implemented later...
| @@ -118,6 +125,12 @@ public void onClick(View v) { | |||
| ReadableMap actionDetails = options.getMap("action"); | |||
| snackbar.setAction(actionDetails.getString("title"), onClickListener); | |||
| snackbar.setActionTextColor(actionDetails.getInt("color")); | |||
|
|
|||
| if (fontFamily != null) { | |||
| Typeface font = Typeface.createFromAsset(this.context.getAssets(), "fonts/" + fontFamily + ".ttf"); | |||
There was a problem hiding this comment.
Is this directory conventional? What about res/font/ for example?
| } | ||
|
|
||
| if (fontFamily != null) { | ||
| Typeface font = Typeface.createFromAsset(this.context.getAssets(), "fonts/" + fontFamily + ".ttf"); |
There was a problem hiding this comment.
Since this is done twice let's make it a variable to cache it.
android/src/main/java/com/azendoo/reactnativesnackbar/SnackbarModule.java
Outdated
Show resolved
Hide resolved
| private static final String REACT_NAME = "RNSnackbar"; | ||
|
|
||
| private List<Snackbar> mActiveSnackbars = new ArrayList<>(); | ||
|
|
||
| public SnackbarModule(ReactApplicationContext reactContext) { | ||
| super(reactContext); | ||
| this.context = reactContext.getApplicationContext(); |
There was a problem hiding this comment.
Instead of saving a member var that could be nullified when the app goes away, let's just use a local Context context = getReactApplicationContext().getApplicationContext() below.
There was a problem hiding this comment.
Sorry, I don't now much about android. so I can't understand where should I put this line.
|
Is there any update on that pr? @alijamaliz @cooperka. I just want to use a custom font family. |
|
Hi @guvenakcoban, I haven't yet had time to fully test this using the new props and RTL language settings, or to document screenshots before/after. I may have time next week around the holidays. In the meantime, you are welcome to fork this library and merge the PR on your fork so you can use it right away. |
|
I will leave this PR open until RTL changes are also in (I'm planning to cherry-pick those as well, but no guarantees). |
|
And
Source:
Closing as these changes are now merged. Released in v2.2.0 🎉 |
No description provided.