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

Fix some issues with stories #6

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

marco2216
Copy link
Contributor

Hi! First off, nice library, it looks great.
We used this for a hackathon for our app to implement stories, and we discovered a few issues, so I hope it's OK that I'm opening a PR with some suggestions for fixes.
I might also have a few UX suggestions/features/refactors that I will suggest in future PRs/issues.

This PR fixes the following:

  1. When renderContent is passed, the close button does not work. Fixed by setting pointerEvents="box-none" on the content view.
  2. Image.prefetch is called even if the story is a video, which gives an error as it cannot decode the Video data.
  3. A bug in onImageChange: when going back to the previous user, the "progress" animation is always reset, but sometimes the image does not change. This is caused by the active state not always being true before the activeStory state changes. I fixed this by removing the early return if active.value is not true. I don't think this check is necessary, since we return immediately after if the story matching the activeStory is not found.

@marco2216
Copy link
Contributor Author

I think there's similar bugs to 3. with some of the other components, like the content, header, maybe progress? Just didn't notice it before, but the same issue is there because of the isActive state race condition.

@marco2216
Copy link
Contributor Author

Perhaps a more general fix for the issue with the isActive state would be to derive the userIndex from the currentStory value, instead of the x position?
Something like this

    const userIndex = useDerivedValue(() =>
      stories.findIndex((user) =>
        user.stories.some((story) => story.id === currentStory.value),
      ),
    )

@Lipo11
Copy link
Contributor

Lipo11 commented Oct 23, 2023

Hello thanks for using and contributing our lib! appreciate it! we will check the active prop problem and let you know, other thinks looks fine.

@Lipo11 Lipo11 self-assigned this Oct 23, 2023
@Lipo11 Lipo11 added the bug Something isn't working label Oct 23, 2023
@Lipo11 Lipo11 merged commit d428477 into birdwingo:main Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants