search

quran / quran.com-frontend

quran.com frontend
https://quran.com
MIT License
990 stars 361 forks source link

Last ayah audio double playing #276

Closed mmahalwy closed 8 years ago

dashohoxha commented 8 years ago

If pause button is pressed while the last ayah is playing, one of the playings is stoped, but the other continues. So, maybe it is triggered automatically, outside the control of the audio player buttons.

mmahalwy commented 8 years ago

yeah i noticed that too

azizur commented 8 years ago

I have just spotted this as well just right now.

mmahalwy commented 8 years ago

anyone want to write a failing test PR?

dashohoxha commented 8 years ago

How do you write a failing test?

azizur commented 8 years ago

@mmahalwy I will give it a go.

mmahalwy commented 8 years ago

@dashohoxha @azizur a PR that will show the tests failing would suffice that way I can write the code to get it to pass. For example, the test would test that 'Loading...' is not present at the end of the surah, which would fail :)

dashohoxha commented 8 years ago

@mmahalwy I don't know how to write such tests, that's why I asked. I have to study first any tutorials on React etc. Do you have any suggestions on where should I start?

But then, a failing test is not a requirement for fixing a bug. I understand that the TDD is the latest silverbullet in software engeenering, but I haven't used it yet, and I can still manage my programs and fix bugs and in general they have no bugs. Besides, writing a test for a bug may be much more difficult than fixing the bug itself, and I seriously doubt that you can write a failing test for each possible bug. I think that TDD makes more sense for features than for bugs. Just my opinion.

dashohoxha commented 8 years ago

Can it be that the problem is with the data returned from API (for example last ayah is returned twice)? By the way, testing an API seems much more easier to me than testing a UI.

mmahalwy commented 8 years ago

okay no problem. I will get to it soon inshallah.

I can write a test as an example - I should probably commit to writing tests for Surah anyways

azizur commented 8 years ago

I have been trying to write some unit tests to get things started. I have few query as to how things are setup on this project.

The Readme file says npm run test:watch to run the tests, but when I run that I am seeing npm ERR! missing script: test:watch because there is no script setup for this.

There are no unit tests currently in the project.

mmahalwy commented 8 years ago

See: https://github.com/quran/quran.com-frontend/blob/master/src/components/LazyLoad/spec.js

and run via npm run test:ci:unit

AhmedShab commented 8 years ago

Salam @mmahalwy, I would be keen to work on that bug. I was looking into Quran site then found yours on the top of the search. Was curious about the development and that's how I found this project.

I have a computer science background and recently/currently I'm gaining experience in the web development (full stack). I don't know much about React but I used AngularJS so It will be easy to learn it :)

I wanna contribute to something valuable like this project, would you please tell me how I can get started on? Thanks

mmahalwy commented 8 years ago

@AhmedShab thanks for binging and we could definitely use your help!

I'd suggest taking a look at the README which will describe how to set up this repo. Since Ramadan is on the horizon, we are trying to scope and focus our time on these tasks: https://github.com/quran/quran.com-frontend/issues/312

I will include this bug as part of it too. Feel free to comment on any issue, we do a good job of keeping up with them, or open a new one if you'd like.

AhmedShab commented 8 years ago

@mmahalwy No problem :+1: and thanks for your quick respond.

I will fork the repo and get started with the README file (so excited :smile: ). Will check these takes and work on them as they are the priority at this time.

I have other commitments (like I'm working/ studying at devacademy) but will spend my free time on it.

azizur commented 8 years ago

I have been looking at this, I will have a work in progress PR shortly.

Just checked on LIVE I am getting: undefined:1 Uncaught (in promise) DOMException: The play() request was interrupted by a call to pause().

Not sure if I should raise another issue for this or not.

mmahalwy commented 8 years ago

@azizur the audioplayer does throw that error but I don't think it's the cause of the double play.

mmahalwy commented 8 years ago

Fixed by #335