Add getStatusBarHeight method for Android
#98
Conversation
…on Android only at the moment)
|
What is the purpose of the method? At what point do you need this information in your app? If anything, I would have expected to see this in the iOS part. |
|
When you use |
There was a problem hiding this comment.
I'm not sure about this PR. Isn't the Android status bar usually 24dp? In practice that would always be 24px in a Cordova app, and it has usually worked well for me. Having a function to calculate it could be useful, though it's possible this implementation wouldn't work in all cases (e.g. split screen). What do you think?
www/statusbar.js
Outdated
| exec(function (result) { | ||
| successCallback(result); | ||
| }, errorCallback, "StatusBar", "getStatusBarHeight", []); | ||
| StatusBar.isVisible = true; |
There was a problem hiding this comment.
This looks like it's left over from a copy-paste of show
src/android/StatusBar.java
Outdated
| Rect rectangle = new Rect(); | ||
| Window window = cordova.getActivity().getWindow(); | ||
| window.getDecorView().getWindowVisibleDisplayFrame(rectangle); | ||
| int statusBarHeight = Math.round(rectangle.top / Resources.getSystem().getDisplayMetrics().density); |
There was a problem hiding this comment.
If the app is on the bottom of a split-screen, will this return the offset from the top of the screen? If so, this value will be incorrect.
There was a problem hiding this comment.
Good catch. I see the issue and im trying to figure out an fix for this.
|
I've pushed changes that fixes split mode issue - If we detect that we're in split mode at the bottom, we simply return height |
|
@aszmyd Looking at the Android code, the statusbar height is always |
|
@Menardi i think you're right. I've pushed a change with constant and some readme updates. Please check out. |
|
@aszmyd I was was more thinking that since it's always 24px, there's no need for the |
|
@Menardi i thought about that too but how would user app detect split mode issue? If user statically set 24px then even in split mode it'll use that. |
|
True, but I think that is a different scenario in a way. I'm not sure Cordova apps even officially support split screen mode. If it is supported, I think it would make more sense for the Cordova core to handle the split screen change, rather than this plugin. Then the user could handle this scenario themselves. |
|
The main issue is that when statusbar is transparent and overlapping webview, app does not know how much space it needs to leave in order for everything too look sane. I think this approach (with resulting |
|
Personally I think the split-screen logic should be in the core, because there are more use cases for it apart from the status bar. So, I don't think there is a need for |
|
In 4.4, the status bar height is 25 dp. And I don't believe it's guaranteed not to change. |
|
Ok i understand your concerns. Thanks for your time. I'm closing this PR |
|
On my One Plus 6 the status bar is 30px in cordova. It's definitly not always 24px. Can this be reopenend? |
Platforms affected
Android
What does this PR do?
Added StatusBar.getStatusBarHeight method (Android only) that gets current statusbar height in CSS pixels
What testing has been done on this change?
Manual testing on Android device