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

[Gallery] feat: Added onZoomBegin and onZoomEnd callbacks for gallery #38

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

HadesShadows
Copy link
Contributor

@HadesShadows HadesShadows commented Jul 20, 2024

I was the one who raised the issue, this is a code that I have written for gallery. I have not used other components therefore not added these callback support there as I haven't seen the code

These callbacks will support index: number to allow developers to have more control on the zoom event

What is added?

Name Type Default Description
onZoomBegin (index: number) => void undefined Callback triggered when component is zoomed.
onZoomEnd (index: number) => void undefined Callback triggered when component returns back to original state.

How it works?

useAnimatedReaction(
    () => scale.value,
    (value, previousValue) => {
      if (value !== 1 && !hasZoomed.value) { //if the value != 1, that means it is not in default state
        hasZoomed.value = true;
        onZoomBegin && runOnJS(onZoomBegin)(activeIndex.value);
      } else if (value === 1 && previousValue !== 1 && hasZoomed.value) { 
        /* 
          when the component is initally rendered, the value will be 1 and prevValue
            will not be 1, this condition will be true, and onZoomEnd() will be triggered
            to counter that, hasZoomed is used 
        */
        hasZoomed.value = false;
        onZoomEnd && runOnJS(onZoomEnd)(activeIndex.value);
      }
    },
    [scale]
  );

Note: I haven't tested the code snippet in iOS since I don't have macOS, but it is working as expected in android

@Glazzes
Copy link
Owner

Glazzes commented Jul 20, 2024

Hi there, I'll not have time to review and test your changes until monday because of personal reasons, but I will be sure to test it as soon as I can.

@HadesShadows
Copy link
Contributor Author

No Problem, take your time

@cw-nikhilnaik
Copy link

Whats the status on the pr?

@Glazzes
Copy link
Owner

Glazzes commented Jul 28, 2024

@cw-nikhilnaik Pending, I've been working on another issue these days and personal issues are a thing aswell.

@Glazzes
Copy link
Owner

Glazzes commented Aug 1, 2024

@HadesShadows Hi there, I just tested your changes against one of the newest features I've made and it works pretty well with it, I'm merging it 👍

I will release a new version soon!

@Glazzes Glazzes merged commit 06ae1ef into Glazzes:main Aug 1, 2024
4 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.

3 participants