Move song status to store #99

Andronikus · 2020-01-06T13:18:15Z

Display Trello task link

Features/Libraries

Redux Store updated
Added to Song object in Redux Store the following attributes: duration, currentTime and isPlayPressed.

This state was managed internally in App.js and PlayBar.js. Now the state is in Redux Store and its is managed in App.js (dispatch actions) only.

Merging development changes, to master branch.

Merge development to master

Merging Development changes to master

YogiHa · 2020-01-11T07:27:01Z

Hi @Andronikus and thanks for your time and contribution :)

For me to be able to review your PR agianst the current project, I need you to fetch the upstream by -

git checkout development - visit development branch
git fetch upstream development - get the newest changes from the development branch.
git merge upstream/development - merge development branch to your own local branch.
git checkout moveSongStatusToStore - visit your current branch
git merge upstream/development - merge development branch changes with your current changes.

after that run
$ git status
for check files that both you and other members modified for manually adjustment.
( on the way, you can delete { route } at initialState.js that is useless code pls :) )
and then push again.

BTW, lovely done on App.js and AppBrowser.js for making whole app not render by redux constantly!

YogiHa · 2020-01-11T07:45:06Z

src/utils/timeConversion.js

@@ -0,0 +1,23 @@
+export function getSecondsFromTimeFormated(time){


export const getSecondsFromTimeFormated = time =>

removed this function. With later developments it will not be needed anymore

YogiHa · 2020-01-11T07:47:10Z

src/store/store.js

 import { reduxFirestore, getFirestore } from 'redux-firestore';
 import FBConfig from '../firebase/config';

+const composeEnhancers = (process.env.NODE_ENV === 'development' ? window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ : null) || compose;
+
 export const store = createStore(
    rootReducer,
-    compose(
+    composeEnhancers(
        applyMiddleware(thunk.withExtraArgument({ getFirebase, getFirestore })),
        reactReduxFirebase(FBConfig, {
            userProfile: 'users',


you dont need to push this file to git

Its true! Will kept it in my local branch only

YogiHa · 2020-01-11T07:47:58Z

src/store/reducers/songReducer.js

        case 'SONG_CLEARED':
            return {
                downloadState: false,
-                song: {
-                    url: null,
-                    name: null,
-                    blob: null
-                }
+                song: initialState.song


watch for the conflict here

Already resolved!

YogiHa · 2020-01-11T07:49:41Z

src/pages/App/AppBrowser.js

+    // const song = useSelector(state => state.song);
+    const songUrl = useSelector(state => state.song.url);
+
+    return (
+        <div>
+            {uid ? (
+                <div>{songUrl ? <App /> : <UploadSong />}</div>
+            ) : (
+                <Redirect to="/" />
+            )}
+        </div>


in this file a first chat functionality added,
supposedly it don't affect the same changes you did at that file, but be careful not delete those

Already merged this too! Was careful not to delete changes already added

YogiHa · 2020-01-11T07:57:49Z

src/pages/App/App.js

+        const currentTime = getTimeFormated(e.target.currentTime);
+        if(!songCurrentTime || songCurrentTime !== currentTime){
+            dispatch(setCurrentTimeFormated(currentTime));
+        }


in later versions of this file, there was some other workaround regard song time, that implemented directly on { onTimeUpdate } method of the audio ref.
just change there the { setCurenntTIme } function in the end to go through redux and delete this function and I think it is gonna work fine.

YogiHa · 2020-01-11T08:00:29Z

src/components/PlayerBar/PlayerBar.js

-    // Set formated duration
-    useEffect(() => {
-        setFormatedDuration(getTime(duration));
-    }, [duration]);
-
-    // Set formated current time
-    useEffect(() => {
-        setFormatedCurrentTime(getTime(currentTime));
-    }, [currentTime]);
-
-    // Format time (eg. 140 to 2:20)
-    const getTime = dur => {
-        return (
-            Math.floor(dur / 60) + ':' + ('0' + Math.floor(dur % 60)).slice(-2)
-        );
-    };
-


I see that you delete those and created a songUtil, at App.js { OnTimeUpdate } at the audio ref at later version of the code there is usage of parse.int that also can be refracted

song duration & song current time will be in sec at redux store so I will keep the util file to convert the seconds in a string.

…to redux store

YogiHa · 2020-01-17T13:40:53Z

src/pages/App/App.js

    // Local States
-    const [playPressed, setPlayPressed] = useState(false);
+    const [uploadedSong, setUploadedSong] = useState(null);


This piece of state has been deleted because it is just a duplication of redux song state. Pls don't re-add it

YogiHa · 2020-01-17T13:45:11Z

src/pages/App/App.js

                            isPlaying={isPlaying}
-                            uploadedSong={song}


As for comment above,
You can go to PlayerBar.is and there change the reference of { uploaded song } prop to redux store state, and on the way delete few other props as { playPressed, onPlayPress} etc..

True! Already did some cleaning in PlayerBar.js and Visualizer.js and App.js

YogiHa · 2020-01-17T13:47:21Z

src/store/reducers/songReducer/songReducer.js

-                name: null,
-                blob: null
-            };
+            return initialState.song;


YogiHa · 2020-01-17T15:29:12Z

whole server dir don't exist at your branch,
please git add it to

ArnasDickus and others added 6 commits November 11, 2019 08:46

Merge pull request zero-to-mastery#71 from zero-to-mastery/development

330f8b5

Merging development changes, to master branch.

Update project.json

7db83c6

Merge pull request zero-to-mastery#85 from zero-to-mastery/development

bd03b54

Merge development to master

Merge pull request zero-to-mastery#87 from zero-to-mastery/development

2ec9d9e

Merging Development changes to master

add song current_time, duration and isPlaying to store

58989e7

fix send isPlayPressed status in onSongEnd

6668002

YogiHa suggested changes Jan 11, 2020

View reviewed changes

Andronikus added 2 commits January 14, 2020 16:29

merge devs with moving duration/currentTime & playPressed song state …

76e1dee

…to redux store

merge with development branch

6b0c0ec

YogiHa suggested changes Jan 17, 2020

View reviewed changes

Andronikus added 2 commits January 17, 2020 23:25

use only song info from redux store in App.js

b79198d

change some song local state to sonf state in redux store

ae61b0d

YogiHa merged commit 78138b0 into zero-to-mastery:development Jan 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move song status to store #99

Move song status to store #99

Andronikus commented Jan 6, 2020

YogiHa commented Jan 11, 2020

YogiHa Jan 11, 2020

Andronikus Jan 14, 2020

YogiHa Jan 11, 2020

Andronikus Jan 14, 2020

YogiHa Jan 11, 2020

Andronikus Jan 14, 2020

YogiHa Jan 11, 2020

Andronikus Jan 14, 2020

YogiHa Jan 11, 2020

YogiHa Jan 11, 2020

Andronikus Jan 14, 2020

YogiHa Jan 17, 2020

Andronikus Jan 17, 2020

YogiHa Jan 17, 2020

Andronikus Jan 18, 2020

YogiHa Jan 17, 2020

YogiHa commented Jan 17, 2020

		@@ -0,0 +1,23 @@
		export function getSecondsFromTimeFormated(time){

Move song status to store #99

Move song status to store #99

Conversation

Andronikus commented Jan 6, 2020

Display Trello task link

Features/Libraries

YogiHa commented Jan 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YogiHa commented Jan 17, 2020