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.

Sign up for GitHub

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

Jump to bottom

Adds target control #301

Merged
merged 6 commits into from
Jun 3, 2024
Merged

Adds target control #301

merged 6 commits into from
Jun 3, 2024

Conversation

dkastl
Copy link
Member

@dkastl dkastl commented May 31, 2024

Fixes #300 .

Changes proposed in this pull request:

  • Adds target control
  • Adds plugin settings to enable/disable target control

dkastl added 2 commits June 1, 2024 00:39
@dkastl
Adds permalink control
7cea6e5 
Fixes #12

Signed-off-by: Daniel Kastl <[email protected]>
@dkastl
Target cross control
43d460b 
Fixes #300

Signed-off-by: Daniel Kastl <[email protected]>
@dkastl dkastl added this to the v5.1.0 milestone May 31, 2024
@dkastl dkastl requested review from smellman and sanak May 31, 2024 16:11
@dkastl dkastl self-assigned this May 31, 2024
@dkastl
Fixes radius
73cd920 
Signed-off-by: Daniel Kastl <[email protected]>
sanak
sanak requested changes Jun 2, 2024
View reviewed changes
Copy link
Member

@sanak sanak left a comment

Choose a reason for hiding this comment

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

I confirmed that it works correctly on my PC (macOS) Chrome,
but the target control size was larger on mobile iPhone Safari and Android Chrome.
It would be nice to have same size as pc in mobile browsers.

PC (macOS+Chrome) iPhone (Safari) Android (Chrome)
gtt-target-control-on-pc gtt-target-control-on-iphone gtt-target-control-on-android

@dkastl
Copy link
Member Author

dkastl commented Jun 2, 2024

I see. Thanks for checking on mobile. I didn't do this.

Since this comes from ol-ext, I'm not sure if this can be solved without upstream changes, but I will try. It actually uses OpenLayers style to draw the plus sign, so I'm wondering how ol.style examples look on mobile devices.

@dkastl
Copy link
Member Author

dkastl commented Jun 2, 2024

This adjustment should solve the scaling on high DPI devices: 6814209

@dkastl dkastl requested a review from sanak June 2, 2024 18:03
sanak
sanak approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@sanak sanak left a comment

Choose a reason for hiding this comment

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

Thanks!
I confirmed that target control size is good in PC Chrome, iPhone Safari and Android Chrome.

@dkastl dkastl merged commit 4b4de0c into next Jun 3, 2024
10 checks passed
@dkastl dkastl deleted the dkastl/issue300 branch June 3, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanak sanak sanak approved these changes

@smellman smellman Awaiting requested review from smellman

Assignees

@dkastl dkastl

Labels
None yet
Projects
Release v5.x
Archived in project
Milestone
v5.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dkastl @sanak