This repository has been archived by the owner on Jun 28, 2021. It is now read-only.
Fix audio player bar not loaded when changing chapter #854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
August 8, 2017 23:10
- Fix currentVerse undefined - Restart currentTime to 0 when chapter changed
|
rebuild |
1 similar comment
|
rebuild |
|
@imadmk Jenkins will soon generate an environment for this pr, so we can test and I'll approve the pr. thank you |
|
Deployed to: http://staging.quran.com:32933 |
thabti
approved these changes
Aug 23, 2017
|
Deployed to: http://staging.quran.com:32934 |
naveed-ahmad
suggested changes
Aug 23, 2017
| @@ -447,7 +447,7 @@ class Surah extends Component { | |||
| <Audioplayer | |||
| chapter={chapter} | |||
| verses={verses} | |||
| currentVerse={verses[currentVerse]} | |||
| currentVerse={verses[currentVerse] || verses[Object.keys(verses)[0]]} | |||
There was a problem hiding this comment.
I guess we're doing the same thing in mapStateToProps, currentVerse shouldn't be nil.
Also this will fix the audio player what about other components which are using currentVerse ? see renderVerses and renderLines for example. I believe we should fix mapStateToProps and make sure current verse isn't undefine at all.
|
@imadmk like I said Fixed. And closing this. |
|
I see. Great it fixed. Because that will fix many UI flows. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Assalamualaikum
Bismillah this is my first contribution.
Insha Allah this commit will fix issue 815 and 830 and maybe 675 too
Fix currentVerse undefined
I found when user change verse, the currentVerse is not changed. So if user already on 4th verse on Surah 1 and move to Surah 3 for example, it will cause Audioplayer breaks (on staging, and endless loading on production) because
ownProps.currentVerseis undefined onfiles[ownProps.currentVerse.verseKey](on Audioplayer/index.js).This is caused by undefined
verses[currentVerse]' on 'Surah/index.jsupon rendering Audioplayer.When new surah loads,
verseswill be assigned bystate.verses.entities[chapterId]so it will be filled with {3:1, 3:2, 3:3 ...}.So it will be undefined since
currentVerseis still on1:4and verses[1:4] does not exist.This fix will assign
currentVerseto first element from verses when it is undefined.Restart currentTime to 0 when chapter changed
After audio player succesfully loaded, the state of
currentTimedid not change. So if the player already played to half of the ayah, it won't reset to0in new Surah.This fix will reset the
currentTimeto 0 every unmounting Audioplayer element.