-
Notifications
You must be signed in to change notification settings - Fork 157
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
[ornaments] Refactor visibility options for the logo and attribution button #326
Conversation
36efe8b
to
666f045
Compare
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.
Looks good - question about visibility
vs _visibility
public override var isHidden: Bool { | ||
didSet { | ||
if isHidden { | ||
Log.warning(forMessage: "Attribution must be enabled if you use data from sources that require it. See https://docs.mapbox.com/help/getting-started/attribution/ for more details.", category: "Ornaments") |
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'm assuming this language was checked by @sbma44
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.
PRs must be submitted under the terms of our Contributor License Agreement CLA.
Fixes: < Link to related issues that will be fixed by this pull request, if they exist >
Pull request checklist:
mapbox-maps-ios
changelog:<changelog></changelog>
.Summary of changes
The
_isVisible
property on the logo view and attribution button's ornament options is nowvisibility
. This adds consistency with the compass and scale bar'svisibility
property.Refactor
OrnamentOptions
so that there is oneBaseOrnamentOptions
struct, rather than individual structs for each ornament.This PR also adds an
Log.error
that is shown when the logo or attribution button is hidden.User impact (optional)