Rewrite on top

[mmahalwy]

Rewrite

Goals

• New tech: move from Fluxible to Redux -> it's a more adopted and better framework for data storage. Good for oncoming devs to help.
• Faster: added static data -> there is a lot of data we almost never change and I think it could be on the frontend
• Better architecture: more components and everything is bite-sized, better for testing which I never did much of
• Flexible: can easily add authentication and other features. Especially with Redux things are easy to change, to add.
• Better Express: relying more on Express for relaying calls, etc.

Notes

• The files change is huge: because I changed the folder structure and the fonts files are moved to a new location
• The additions and subtractions are huge: again, same problem as before.

Questions

• Why a rewrite already? 1. Fluxible is dying and I don't want to get stuck. 2. Reactjs devs probably use or played around with Redux. 3. Future features will be easier to implement with Redux
• Why didn't you write in Redux to begin with? It didn't exist when I wrote this.
• Why are tests failing? I didn't write any yet.
• When will this be finished? I hope to ASAP. Maybe in a month inshallah

[mmahalwy]

need to readd this!

[azizur]

Re-write the entire app? Files changed 2,763? This makes it impossible for any decent code review.

In future, it would be nice to have smaller change set and many of them. Small change makes it easy to review and also stop proceeding in a direction before it too late.

The PR also has no description so I have no idea what this big change is meant to archive and where the project is heading.

CI tests are also failing.

[mmahalwy]

@azizur thanks for the comment! I think the size of the change is simply due to font files being stored in this repo, so if you change their folder location, you changed 1000s of files. I am contemplating the rewrite now as it's taking me too too long to finish and I need more time to spend thinking about it. I'll add a description too

[azizur]

Just pulled this branch and run it locally.

I am getting this error below was this expected?

==> 💻  Open http://localhost:8000 in a browser to view the app.
MIDDLEWARE ERROR: [TypeError: action.result.result.map is not a function]
{ surahId: 'public',
  error: [TypeError: action.result.result.map is not a function],
  type: '@@quran/ayahs/LOAD_FAIL' }
{ surahId: 'public',
  error: [TypeError: action.result.result.map is not a function],
  type: '@@quran/ayahs/LOAD_FAIL' }
DATA FETCHING ERROR:   TypeError: Cannot read property 'ayat' of undefined

  - index.js:26 _dec.loadAyahsDispatch
    index.js:26:45

  - connect.js:124 Connect.configureFinalMapState
    [quran.com-frontend]/[react-redux]/lib/components/connect.js:124:27

  - connect.js:114 Connect.computeStateProps
    [quran.com-frontend]/[react-redux]/lib/components/connect.js:114:23

  - connect.js:156 Connect.updateStatePropsIfNeeded
    [quran.com-frontend]/[react-redux]/lib/components/connect.js:156:35

  - connect.js:267 Connect.render
    [quran.com-frontend]/[react-redux]/lib/components/connect.js:267:40

  - ReactCompositeComponent.js:587 [object Object].ReactCompositeComponentMixin._renderValidatedComponentWithoutOwnerOrContext
    [quran.com-frontend]/[react]/lib/ReactCompositeComponent.js:587:34

  - ReactCompositeComponent.js:607 [object Object].ReactCompositeComponentMixin._renderValidatedComponent
    [quran.com-frontend]/[react]/lib/ReactCompositeComponent.js:607:32

  - ReactPerf.js:66 [object Object].wrapper [as _renderValidatedComponent]
    [quran.com-frontend]/[react]/lib/ReactPerf.js:66:21

  - ReactCompositeComponent.js:220 [object Object].ReactCompositeComponentMixin.mountComponent
    [quran.com-frontend]/[react]/lib/ReactCompositeComponent.js:220:30

  - ReactPerf.js:66 [object Object].wrapper [as mountComponent]
    [quran.com-frontend]/[react]/lib/ReactPerf.js:66:21

[azizur]

[azizur]

Refactoring like this without any test is bit worrying.

Better architecture: more components and everything is bite-sized, better for testing ...

We need to start with test, then build it up from there. Leaving Test to last is going result in sub standard quality.

Can we split up this big refactor into smaller one that we we can get more people working on smaller chunk to help and push it forward. Rather then only one person taking a lengthy re-write task?

How can I help?

[azizur]

Issue I found in this PR:

• When audio is playing: Add a second translation, it causes a second audio playback. Same for removing a translation.
• Play button next to Copy button on ayah does not work.
• Copy button on ayah only copies Arabic string... but icon next to it indicated by paper-clip.
• When audio is playing: Interacting with Change Font Size controls create infinite loop of audio playback on the same ayah.
• Api gave a bad response:

GET /api/surahs/75/ayat?from=40&to=50&audio=8&quran=1&content%5B%5D=19 304 48.542 ms - -
proxy error { [Error: getaddrinfo ENOTFOUND quran.com quran.com:3000]
  code: 'ENOTFOUND',
  errno: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'quran.com',
  host: 'quran.com',
  port: '3000' }
GET /api/surahs/75/ayat?from=40&to=50&audio=8&quran=1&content%5B%5D=19 500 12.908 ms - -

• When audio is playing: Scrolling to the end of surah create infinite loop of audio playback on the last ayah.
• horizontal scrolling possible on selecting surahs selection list.

Given all these issue I found I considering this PR is not ready for review/testing.

[azizur]

React is throwing warning on console log.

Also there appears to be console log message in there.

[mmahalwy]

@azizur thanks for the comments. I havent finished this yet. It's in PR so that people can take a look as I progress through it. That being said, I am going to close this PR and work off of master for smaller chunks

[mmahalwy]

In response to this comment #187 (comment), I do encourage having PRs even if they are not ready then flag them as ready for review. That way we can get feedback as the author is progressing rather than 1 full review at the end (which has to happen, but it's better to get context along the way).