Skip to content

Conversation

@n8han
Copy link
Collaborator

@n8han n8han commented Apr 12, 2021

This override allows the application to provide a custom UX for Mapbox
telemetry settings and all required copyright information.

This improves the utility of the existing getTelemetryEnabled and
setTelemetryEnabled methods, as there is otherwise no means to provide
an alternative to the basic action sheet shown by default.

While it seemed a bit dubious to alter the click target of a UI element
that belongs to the underlying framework, it's apparently expected that
the application may override the button's visibility as that use case is
handled by the framework. (The application is prevented from running
unless a particular property is added to the info.plist.)

I explored that route as well, hiding the attribution button and
replacing it with an imitation, however the built-in button has features
that would be difficult to replicate in an externally added view. For
example, it repositions itself when the content insets change, and it's
always properly aligned with the Mapbox logo to the left.

Since the appearance and positioning of the built-in button is good and
appropriate for almost any application, but the action sheet is rather
basic and won't include whatever additional attributions an application
may require, overriding its click action appears the be the best option.

@n8han n8han added enhancement New feature or request ios needs Android labels Apr 12, 2021
@n8han n8han requested a deployment to ANDROID_CI_DOWNLOADS_TOKEN April 12, 2021 22:54 Waiting
@n8han n8han requested a deployment to ANDROID_CI_DOWNLOADS_TOKEN April 12, 2021 22:54 Waiting
@n8han n8han requested a deployment to ANDROID_CI_DOWNLOADS_TOKEN April 27, 2021 01:11 Waiting
@n8han n8han requested a deployment to ANDROID_CI_DOWNLOADS_TOKEN April 27, 2021 01:11 Waiting
@tobrun tobrun force-pushed the AttributionButtonHandler branch from 0c60333 to ed1bd2f Compare June 5, 2021 07:03
@tobrun tobrun had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 07:03 Failure
@tobrun tobrun had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 07:03 Failure
@tobrun tobrun force-pushed the AttributionButtonHandler branch from ed1bd2f to b9083ec Compare June 5, 2021 07:23
@tobrun tobrun had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 07:23 Failure
@tobrun tobrun had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 07:23 Failure
This override allows the application to provide a custom UX for Mapbox
telemetry settings and all required copyright information.

This improves the utility of the existing `getTelemetryEnabled` and
`setTelemetryEnabled` methods, as there is otherwise no means to provide
an alternative to the basic action sheet shown by default.

While it seemed a bit dubious to alter the click target of a UI element
that belongs to the underlying framework, it's apparently expected that
the application may override the button's visibility as that use case is
handled by the framework. (The application is prevented from running
unless a particular property is added to the info.plist.)

I explored that route as well, hiding the attribution button and
replacing it with an imitation, however the built-in button has features
that would be difficult to replicate in an externally added view. For
example, it repositions itself when the content insets change, and it's
always properly aligned with the Mapbox logo to the left.

Since the appearance and positioning of the built-in button is good and
appropriate for almost any application, but the action sheet is rather
basic and won't include whatever additional attributions an application
may require, overriding its click action appears the be the best option.
@tobrun tobrun force-pushed the AttributionButtonHandler branch from b9083ec to efe23b7 Compare June 5, 2021 09:04
@tobrun tobrun temporarily deployed to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 09:04 Inactive
@tobrun tobrun temporarily deployed to ANDROID_CI_DOWNLOADS_TOKEN June 5, 2021 09:04 Inactive
@tobrun
Copy link
Collaborator

tobrun commented Jun 5, 2021

@n8han thank you for the contribution, I updated this PR with null safety support.
Will merge once CI approves and ticket out tail work for Android.

@tobrun tobrun merged commit f90d54d into master Jun 5, 2021
@n8han
Copy link
Collaborator Author

n8han commented Jun 5, 2021 via email

@n8han n8han deleted the AttributionButtonHandler branch October 30, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants