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

Expose setPos and setZoom to ref #70

Open
fkrauthan opened this issue May 21, 2023 · 12 comments
Open

Expose setPos and setZoom to ref #70

fkrauthan opened this issue May 21, 2023 · 12 comments

Comments

@fkrauthan
Copy link
Contributor

I have a use-case where I need to sync a couple of PrismaZoom objects. For that I need to be able to set position and zoom with fixed values (base on the event listeners). In older versions I was able to just use setState directly. But the new version doesn't have that capability anymore.

@sylvaindubus
Copy link
Owner

Hello Florian. Sorry for the delay. Yeah, there were some changes, but you can still affect positions and zoom levels through a ref object. Here are the available methods: getZoom, zoomIn, reset, move, zoomOut, zoomToZone.

@fkrauthan
Copy link
Contributor Author

Yeah but that methods don't work if you try to sync the exact zoom position and level on multiple PrismaZoom objects. I need the raw position and zoom level set and get methods exposed.

@sylvaindubus
Copy link
Owner

The small demo app is using both getZoom from the ref and onZoomChange prop to update a percentage. Shouldn't it could work for you in your case?

@fkrauthan
Copy link
Contributor Author

No as again I don't need the values (they are easy to get) but I have 3 identical sized images in three tabs and when I do any type of operation in one (zoom or move) I need the exact same zoom and move applied to the other two images so that if the user switches the tab he always sees the same part of the image.

@sylvaindubus
Copy link
Owner

Ok, you can try with the latest version 3.3.4, I've exposed setZoom and setPos methods:

const onZoomChange = useCallback((zoom: number) => {
   ref.current?.setZoom(zoom)
 }, [])

 const onPanChange = useCallback(({ posX, posY }: { posX: number; posY: number }) => {
   ref.current?.setPos([posX, posY])
 }, [])

Keep in mind that onPanChange doesn't have the same signature than setPos. I didn't want to introduce breaking changes at this time but it can makes things a bit confusing right now.

@fkrauthan
Copy link
Contributor Author

@sylvaindubus Sorry for the super delayed response. Unfortunately the current implementation of setPost and setZoom does not really work because it refires the onZoomChange and onPanChange events which results in an endless loop. I don't think that firing the events really is required as they obviously change when those methods are called. Maybe we can ether have a boolean that disables event firing or something like that?

@sylvaindubus
Copy link
Owner

@fkrauthan I've investigated a bit on that, and it doesn't seem very easy to add a boolean to disable these events because these functions are also used internally (by zoomIn, zoomOut for instance). The ideal solution would be to create specific open methods for that, which could have a boolean as an argument, but it would make another breaking change, which I would prefer to avoid for now.

In the meantime, maybe you could use a boolean on your side (setting it to true before a manual action, and back to false afterwards), to control whether these functions should have any effect?

@fkrauthan
Copy link
Contributor Author

I don't understand where the complications are?

const setPos = useCallback((pos: PositionType, fireEvent: bool = true) => {

is all you need to change and then if fireEvent is true and onPanChange is defined call it. That should nether break existing logic nor be a breaking change. Using a boolean outside is very ugly and not very easy todo (given that I sync multiple images).

@sylvaindubus
Copy link
Owner

If you check other methods, you will see that zoomIn and zoomOut are calling setZoom, fullZoomInOnPosition is calling setPosand zoomToZone is calling both. It would require adding these booleans everywhere to make it consistent, which doesn't sound good.

So, in the meantime, using an external boolean to do whether something or not with your event is still the "least bad" solution to me. But we should definitely expose different setters than these internal methods for the next major release.

@fkrauthan
Copy link
Contributor Author

Not really as zoomIn and zoomOut as well as fullZoomInOnPosition change the value (as you don't SET the value). Only setPos and setZoom require the optional boolean to skip the event propagation as you specify absolute values.

@sylvaindubus
Copy link
Owner

Hmm, I still don't like it that much, feel a bit hacky to add it. I'll think about another solution and see if something better pops into my mind!

@fkrauthan
Copy link
Contributor Author

alternative you can expose as external API setter that don't call the callback and keep internal the once that does (duplicate some logic)

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

No branches or pull requests

2 participants