Skip to content

[Update] Adjust Compass API signature #154

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

Merged
merged 3 commits into from
Apr 5, 2023
Merged

[Update] Adjust Compass API signature #154

merged 3 commits into from
Apr 5, 2023

Conversation

yo1995
Copy link
Collaborator

@yo1995 yo1995 commented Apr 4, 2023

Description

This PR updates Set viewpoint rotation.

The autohide setting is now a view modifier.

Linked Issue(s)

How To Test

The sample behaves exactly the same as before.

To Discuss

To match the old code, should we explicitly write .autoHideDisabled(true)? I don't think so because…

  1. its default value is true (which means the compass doesn't autohide)
  2. there are other view modifiers on the compass. A user can explore them with other methods, while the sample can keep simple.

@yo1995 yo1995 requested review from mhdostal and dfeinzimer April 4, 2023 23:49
@yo1995 yo1995 self-assigned this Apr 4, 2023
Copy link
Contributor

@dfeinzimer dfeinzimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just the one suggestion from Mark and this should be good to go. Note true can be omitted thanks to the presence of a default value.

The autohide setting is now a view modifier.
@yo1995 yo1995 force-pushed the Ting/UpdateCompass branch from 827c898 to 88f4e93 Compare April 5, 2023 16:55
@yo1995 yo1995 requested review from dfeinzimer and mhdostal April 5, 2023 16:56
Copy link
Contributor

@dfeinzimer dfeinzimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably single line the init since it only has 1 param now

Compass(viewpointRotation: $rotation)
    .autoHideDisabled()
    .padding()

@yo1995 yo1995 requested a review from dfeinzimer April 5, 2023 17:24
@yo1995 yo1995 merged commit 4ad08bc into v.next Apr 5, 2023
@yo1995 yo1995 deleted the Ting/UpdateCompass branch April 5, 2023 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants