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

Chore: Moved PanZoom functions to "web/js/panzoom.js" #4042

Merged
merged 15 commits into from
May 27, 2024

Conversation

IgorA100
Copy link
Contributor

Moved from Montage, Watch & Event page

@IgorA100 IgorA100 marked this pull request as ready for review May 27, 2024 13:27
@IgorA100
Copy link
Contributor Author

@connortechnology
Please check.

@IgorA100
Copy link
Contributor Author

I added another commit 9ec5c1a
Although it is not related to the transfer of PanZoom functions, it is related to the monitorsSetScale function

@connortechnology
If you are against it, then I can delete the commit and create another PR after this PR is approved

@connortechnology
Copy link
Member

No problem with the scale stuff.
This looks great, I'm just wondering if we shouldn't stick it into it's own .js file instead of skin.js. And only load that file on pages that need it.

@IgorA100
Copy link
Contributor Author

it's own .js file instead of skin.js.

I had this idea.
So is "skins/classic/js/panzoom.js" ok?

@connortechnology
Copy link
Member

yeah although maybe web/js/panzoom.js? It would likely be useful to other skins.

@IgorA100
Copy link
Contributor Author

It would likely be useful to other skins.

Ok

@IgorA100
Copy link
Contributor Author

Done

@IgorA100 IgorA100 changed the title Chore: Moved PanZoom functions here (skin.js) Chore: Moved PanZoom functions to "web/js/panzoom.js" May 27, 2024
If scale == 'fit_to_width', then it is necessary to take into account the width of the screen, not the monitor element, because The monitor has not yet reached the correct size.
@connortechnology connortechnology merged commit faa3ed3 into ZoneMinder:master May 27, 2024
10 of 18 checks passed
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