Skip to content
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

feat(compass): sync compass with globe #336

Merged
merged 6 commits into from
Apr 30, 2020
Merged

feat(compass): sync compass with globe #336

merged 6 commits into from
Apr 30, 2020

Conversation

KatvonRivia
Copy link
Member

No description provided.

@KatvonRivia KatvonRivia requested a review from pwambach April 17, 2020 15:53
.compassIcon svg, .downloadIcon svg
fill: $textWhite
.compass svg
fill: #202B33
Copy link
Contributor

Choose a reason for hiding this comment

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

What color is thta? Wouldn't it be better if the icon defines the default fill color itself and we overwrite only on hover?

padding-left: emCalc(25px)
.compass
margin-right: emCalc(16px)
margin-left: emCalc(25px)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need these strange margins? Can we solve this in a better way with flexbox?

@@ -17,6 +18,7 @@ const GlobeNavigation: FunctionComponent = () => {
const dispatch = useDispatch();
const defaultView = config.globe.view;
const projectionState = useSelector(projectionSelector);
const globecheck = useSelector(globeViewSelector);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use the position from the redux store we only get a new value when the globe movement ends. It would be cooler if the icon rotation updates in realtime. Perhaps we can do a little trick and set the rotation as a css variable in the globe component.

onClick={() => dispatch(setFlyToAction({...defaultView}))}
/>
style={{transform: `rotate(${globecheck.position.longitude}deg)`}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Using the longitude seems wrong because the north arrow of the icon rotates even if the globe's north is up. One of the camera rotation values could work better

@KatvonRivia KatvonRivia merged commit b9975e8 into develop Apr 30, 2020
@KatvonRivia KatvonRivia deleted the feat/compass branch April 30, 2020 13:46
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.

2 participants