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: broken progress bar after repeat #75

Merged

Conversation

kirillzyusko
Copy link
Contributor

@kirillzyusko kirillzyusko commented Mar 1, 2024

To reproduce the issue you need:

  • to have a single track in queue
  • play this track
  • activate repeat while track is playing (in options select track)
  • do not move slider - just wait for end of track 😊

Important

The issue is not reproducible in default example app - for some reasons it happens only with specific tracks, below I added necessary changes that needs to be made in RNTP example app.

A queue (playlist.json):

[
  {
    "url": "https://assets.contentstack.io/v3/assets/blt464811b872cc339a/blt5224b03977c199ae/639d58298f1f170dcb412a75/en-koa-3-6-daily-mood-boost_2022-09-05_03_31_39.m4a",
    "title": "Longing",
    "artist": "David Chavez",
    "artwork": "https://rntp.dev/example/Longing.jpeg",
    "duration": 167
  }
]

QueueInitialTracksService.ts:

import TrackPlayer, { Track } from 'react-native-track-player';

import playlistData from '../assets/data/playlist.json';
// @ts-expect-error – sure we can import this
import localTrack from '../assets/resources/pure.m4a';
// @ts-expect-error – sure we can import this
import localArtwork from '../assets/resources/artwork.jpg';

export const QueueInitialTracksService = async (): Promise<void> => {
  await TrackPlayer.add([...(playlistData as Track[])]);
};
Before After
Screenshot 2024-03-01 at 9 40 37 AM Screenshot 2024-03-01 at 9 37 03 AM

@kirillzyusko kirillzyusko force-pushed the fix/broken-progress-bar-after-repeat branch from 5eb8720 to fd8290c Compare March 1, 2024 08:25
@kirillzyusko kirillzyusko reopened this Mar 1, 2024
@kirillzyusko kirillzyusko marked this pull request as ready for review March 1, 2024 08:46
@kirillzyusko
Copy link
Contributor Author

@dcvz I see that unit-test are failing after this, but I tend to think that it's not a big problem - most likely we just need to add additional mocks or something like that (my experience with unit tests in XCode is limited).

Please, let me know if you are happy to have these changes and if yes, then I'll fix unit tests 👀

@dcvz
Copy link
Collaborator

dcvz commented Mar 25, 2024

Thanks for this @kirillzyusko ! Tests unfortunately are flaky on CI - I wouldn't hold the PR on that, although if you're up for improving them, that would be appreciated!

@dcvz dcvz merged commit 03c988e into doublesymmetry:main Mar 25, 2024
1 check failed
@kirillzyusko
Copy link
Contributor Author

@dcvz thanks for merging! I think the problem here is that after my changes even local tests are failing 🙈

I've tried to spend ~1 hour on fixing them locally, but it was very tricky: when I fixed one test - it was working, when I run them in test suite - it was failing again 🙈

@dcvz
Copy link
Collaborator

dcvz commented Apr 12, 2024

@dcvz thanks for merging! I think the problem here is that after my changes even local tests are failing 🙈

I've tried to spend ~1 hour on fixing them locally, but it was very tricky: when I fixed one test - it was working, when I run them in test suite - it was failing again 🙈

they're quite flaky :( fortunately macOS support was now added, so it makes running the tests /a bit/ more stable. CI is now green.

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