Android View Margin #615
Closed
Android View Margin #615
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ins to behave similarly to ios
…avity and it cant be tested now
felix-ht
reviewed
May 11, 2021
| final List attributionButtonMarginsData = toList(attributionButtonMargins); | ||
| sink.setAttributionButtonMargins(toInt(attributionButtonMarginsData.get(0)), toInt(attributionButtonMarginsData.get(1))); | ||
| final Point point = toPoint(attributionButtonMarginsData, metrics.density); | ||
| sink.setAttributionButtonMargins(metrics.widthPixels - point.x - 56, point.y); |
Collaborator
There was a problem hiding this comment.
where is the 56 coming from? Maybe give it a name?
Collaborator
There was a problem hiding this comment.
👍 on naming constant values.
Additionally 56 is a fixed pixel value and to support devices with different screensizes/pixelratios, we should take the density in account. Could you check the density of your device and update the value accordingly? (eg. density 2, so metrics.widthPixels - point.x - 56 becomes metrics.widthPixels - point.x - (28 * metrics.density)
Author
There was a problem hiding this comment.
Thats the width for the attribution button, I named it
…nt and adjust it to density
Author
|
Theres another problem I'm not sure how to solve, the positions reset itself after the device rotates |
n8han
added a commit
that referenced
this pull request
Nov 2, 2021
This continues the work of PR #615 to fix #261 and #540, and replaces the changed calculation of #681 with a more general solution. The underlying issue here is that the attribution button is left-aligned by default on Android, and right-aligned by default on iOS. The approach taken in #681 where the margin x value is assumed to represent the left margin does not generalize to iOS, where it will be a right margin. Since the alignment / gravity of the button can be configured on each platform, we can address this problem at its source by exposing this configuration through flutter-mapbox-gl. Then on Android, we can apply the button margins same way the compass is positioned, with respect to its gravity. I've tested this on iOS and Android with both bottom left and bottom right attribution buttons. The best option currently is to configure the attribution button to bottom right, where it falls by default on iOS. In this way you will have a similar experience on both platforms with a fully configurable margin. The mapbox logo's alignment/position is also configurable, should anyone want to expose it in future.
felix-ht
pushed a commit
that referenced
this pull request
Nov 4, 2021
* fix(android): allow android logoViewMargins and attributionButtonMargins to behave similarly to ios * fix(convert): compass view margin remain the same since theres the gravity and it cant be tested now * feat(setAttributionButtonMargins): adding variable name to the constant and adjust it to density * Add attribution button gravity, position normally This continues the work of PR #615 to fix #261 and #540, and replaces the changed calculation of #681 with a more general solution. The underlying issue here is that the attribution button is left-aligned by default on Android, and right-aligned by default on iOS. The approach taken in #681 where the margin x value is assumed to represent the left margin does not generalize to iOS, where it will be a right margin. Since the alignment / gravity of the button can be configured on each platform, we can address this problem at its source by exposing this configuration through flutter-mapbox-gl. Then on Android, we can apply the button margins same way the compass is positioned, with respect to its gravity. I've tested this on iOS and Android with both bottom left and bottom right attribution buttons. The best option currently is to configure the attribution button to bottom right, where it falls by default on iOS. In this way you will have a similar experience on both platforms with a fully configurable margin. The mapbox logo's alignment/position is also configurable, should anyone want to expose it in future. * Improve switch default behavior When setting the position of the compass and attribution button, we need to assume the platform default gravity for those elements if no explicit value has been provided. An incorrect default was assumed for the attribution button (copied from the compass), so that is corrected here. The methods for setting gravity do not require a default since they are only called if an explicit gravity value has been provided. Those default cases are removed here. #731 (comment) * Rename "gravity" methods in web implementation This name is not commonly used outside of android. Also, we're converting to the typed position objects as quickly as possible with these changes, rather than passing around the int. #731 (comment) Co-authored-by: Ching Yaw Hao <[email protected]>
Collaborator
|
resolved by #731 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request is to address Android logoViewMargins and attributionButtonMargins so that it can behave similarly to iOS
It is related to these 2 issues
#261 #540