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

Scale map labels by zoom #6604

Merged
merged 1 commit into from Apr 1, 2022
Merged

Scale map labels by zoom #6604

merged 1 commit into from Apr 1, 2022

Conversation

Vultraz
Copy link
Member

@Vultraz Vultraz commented Apr 1, 2022

Simple UX improvement that occurred to me. Thoughts?

Before:
image

After:
image

@stevecotton
Copy link
Contributor

Sounds a good idea, but your two pictures suggest that the scaling ratio doesn't match the zoom ratio.

@Vultraz
Copy link
Member Author

Vultraz commented Apr 1, 2022

Wdym.

@stevecotton
Copy link
Contributor

In one picture, the text is as wide as one side of the castle; in the other it's three times as wide as one side of the castle. So they seem to be experiencing different amounts of zoom.

@Vultraz
Copy link
Member Author

Vultraz commented Apr 1, 2022

I mean... they're not, though.

@Wedge009 Wedge009 added the UI User interface issues, including both back-end and front-end issues. label Apr 1, 2022
@Wedge009
Copy link
Member

Wedge009 commented Apr 1, 2022

I'm not sure, but maybe the images themselves being different dimensions is making comparisons difficult. Perhaps it would be helpful to show a normal 100% view and zoomed-in view, for both 'before' and 'after' states.

In an isolated case like this it might be an improvement. Some scenarios are heavy on the labels, however, like the test scenario and defence of the Kalian. Of course, if labels don't overlap in 100% view, it should be fine when zoomed in as well, but it might be nice to see how those situations fare as well.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented Apr 1, 2022

I mean... they're not, though.

I mean… they are, though. That's literally the point of this change. I think @stevecotton may have missed that the two castles you displayed are the same zoom level. As @Wedge009 said it would be helpful to also show a 100% view as reference.

I'd go further and suggest showing a zoomed-out view as well. Perhaps using a different zoom factor for the text isn't such a bad idea, like scaling by 90% or 80% of the actual zoom factor.

@stevecotton
Copy link
Contributor

stevecotton commented Apr 1, 2022

My mistake, I read it as "before zooming"/"after zooming" rather than "before this change"/"after this change". Having tested, it looks good.

@Vultraz
Copy link
Member Author

Vultraz commented Apr 1, 2022

Zoomed out. You're right, it gets too small.
image

@Vultraz
Copy link
Member Author

Vultraz commented Apr 1, 2022

Made zoom go no lower than 1.0 and fixed the y offset.
image

@Vultraz Vultraz merged commit b3b6d7b into master Apr 1, 2022
@Vultraz Vultraz deleted the scale-map-labels branch April 1, 2022 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants