-
-
Notifications
You must be signed in to change notification settings - Fork 518
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
[Android] Add support for translucent header #575
Changes from all commits
2d5db43
6c14f63
d281ae8
956bb9f
274081c
820dc50
7a245e2
3f1af57
a54b5a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,9 +89,9 @@ public void setTitleColor(ScreenStackHeaderConfig config, int titleColor) { | |
config.setTitleColor(titleColor); | ||
} | ||
|
||
@ReactProp(name = "backgroundColor", customType = "Color") | ||
public void setBackgroundColor(ScreenStackHeaderConfig config, int titleColor) { | ||
config.setBackgroundColor(titleColor); | ||
@ReactProp(name = "backgroundColor", customType = "Color", defaultInt = -1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andreibarabas I am noticing that when I try to set my header background color to "#fff" or "#ffffff" (i.e. white), I end up with a non-white header. If you take a look at the attached photo, you can see the header appears to be gray. Any chance this bug came in with the changes to this file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify, this is when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After some further testing, @andreibarabas it looks like it was the change in ScreenStackHeaderConfig.java There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the "#fff", "#ffffff" and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WoLewicki that fixed the issue! Thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guys, glad you solved it. Thanks! Just now had the time to groom the notifications. |
||
public void setBackgroundColor(ScreenStackHeaderConfig config, int backgroundColor) { | ||
config.setBackgroundColor(backgroundColor); | ||
} | ||
|
||
@ReactProp(name = "hideShadow") | ||
|
@@ -119,6 +119,11 @@ public void setHidden(ScreenStackHeaderConfig config, boolean hidden) { | |
config.setHidden(hidden); | ||
} | ||
|
||
@ReactProp(name = "translucent") | ||
public void setTranslucent(ScreenStackHeaderConfig config, boolean translucent) { | ||
config.setTranslucent(translucent); | ||
} | ||
|
||
@ReactProp(name = "backButtonInCustomView") | ||
public void setBackButtonInCustomView(ScreenStackHeaderConfig config, boolean backButtonInCustomView) { | ||
config.setBackButtonInCustomView(backButtonInCustomView); | ||
|
@@ -134,5 +139,4 @@ public void setDirection(ScreenStackHeaderConfig config, String direction) { | |
// RCT_EXPORT_VIEW_PROPERTY(backTitleFontFamily, NSString) | ||
// RCT_EXPORT_VIEW_PROPERTY(backTitleFontSize, NSString) | ||
// // `hidden` is an UIView property, we need to use different name internally | ||
// RCT_EXPORT_VIEW_PROPERTY(translucent, BOOL) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this change is made here? If we expect "default" to be
-1
maybe we should also set the default in@ReactProp
while defining bgColor:??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is a good idea to add that for extra safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. will take care of it as soon as possible.
There is still an open issue from @kyle-ssg which reproduces only if there is a custom left component. I won't have time earlier than next week to investigate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I think this is not an issue since it is the expected behavior for the title to disappear when there is
headerLeft
component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless of if the headerLeft occurred before / after this work I think this is good to merge, I'll look into this separately and raise something if I think there's an issue, it does seem a bit weird to me (mainly because im majority iOS) though it's definitely not a blocker. @WoLewicki @andreibarabas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On iOS, the situation is kind of similar but for
headerCenter
, where using it makes the title disappear.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. it's fine with me. I added @kmagiera suggestion, so let me know if you need anything else from my side to get this merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am testing these changes out and this line seems to have introduced a bug where I can't set the header background color to "#fff" or "#ffffff".