Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

HARP-15446: Control options are not accessible for screen anchor example #2200

Merged
merged 2 commits into from
May 20, 2021

Conversation

germanz
Copy link
Contributor

@germanz germanz commented May 19, 2021

This change contains several minor fixes to the marker anchors example:

  • Use CSS media queries for limiting the example description so that they don't overlap the controls on devices with smaller screen sizes
  • Replace deprecated API usage
  • Move anchor below control GUI

@germanz germanz changed the title HARP-15446: [Native browser] Control options are not accessible for s… HARP-15446: Control options are not accessible for screen anchor example May 19, 2021
@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #2200 (66d0220) into master (50358bf) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2200      +/-   ##
==========================================
+ Coverage   66.62%   66.64%   +0.02%     
==========================================
  Files         312      312              
  Lines       27748    27748              
  Branches     6201     6201              
==========================================
+ Hits        18486    18492       +6     
+ Misses       9262     9256       -6     
Impacted Files Coverage Δ
@here/harp-mapview/lib/MapMaterialAdapter.ts 92.02% <0.00%> (-0.73%) ⬇️
@here/harp-mapview/lib/VisibleTileSet.ts 79.77% <0.00%> (+1.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50358bf...66d0220. Read the comment docs.

<style>
.message {
position: absolute;
float: right;
Copy link
Contributor

Choose a reason for hiding this comment

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

does it really need the float: right, being already absolutely positioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, you're absolutely right, I fixed it.

Comment on lines 186 to 191
<br /> This example shows how MapView.getScreenPosition works for various cases
<br /> the red square is painted on the screen in screen coordinates, whereas the green cube
<br /> lives in the world.
<br /> Use the arrow keys or the gui to change the geoPosition
<br /> Jump to next worlds with "j" and "l"
`;
Copy link
Contributor

@oleksmarkh oleksmarkh May 19, 2021

Choose a reason for hiding this comment

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

I'd remove those <br /> elements, add proper punctuation and let the browser flow the text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, added paragraphs to separate the two different sections.

@oleksmarkh oleksmarkh self-requested a review May 19, 2021 15:57
Signed-off-by: German Zargaryan <2526045+germanz@users.noreply.github.com>
Copy link
Contributor

@oleksmarkh oleksmarkh left a comment

Choose a reason for hiding this comment

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

reviewed fixes for both HARP-15446 and HARP-15426. Thank you!

@germanz germanz merged commit 3640547 into master May 20, 2021
@germanz germanz deleted the HARP-15446 branch May 20, 2021 09:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants