Pomo 647 add our code to fire base

[motaz99]

No description provided.

[github-actions]

Visit the preview URL for this PR (updated for commit 3c77b0c):

https://the-sol-pomodoro--pr33-pomo-647-add-our-cod-csiyyvr0.web.app

_{(expires Tue, 27 Jul 2021 10:27:47 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[mujz]

Great work man! I left you some comments, most of them are very small and will take you no time to address. Let me know when this is ready for review again by re-requesting review.

[mujz]

It doesn't look like you're using this. You should remove it using the command

npm remove react-firebase-hooks

[mujz]

Is this necessary? You should not need this since you're adding the npm package for firebase, right?

[mujz]

Oops, I committed this by mistake. Can you remove this? Sorry 😬

[mujz]

Give this a better name

Suggested change

	const subFunc = (doc) => {
	const handleSnapshotChange = (doc) => {

[mujz]

There is a warning in the browser console saying that you should change this code to

Suggested change

	setState({
	...state,
	periodCounter,
	currentPeriod: periodTime,
	startTime,
	stopTime,
	isRunning,
	isLoading: false,
	timeToShow,
	});
	setState((prevState) => ({
	...prevState,
	periodCounter,
	currentPeriod: periodTime,
	startTime,
	stopTime,
	isRunning,
	isLoading: false,
	timeToShow,
	}));

[mujz]

Improve the documentation by making it say something like this

Suggested change

	* A function that take time as two parameters
	* @param {number} startTime time in millisecond
	* @param {number} stopTime time in millisecond
	* @returns {array} return two time in millisecond as an array
	* A function that determines the value of startTime at the moment the Pomodoro clock starts, and always returns null for the new stopTime because when a Pomodoro clock starts, the stopTime must be reset to empty.
	* @param {number} startTime previous start time in milliseconds
	* @param {number} stopTime previous stop time in milliseconds
	* @returns {array} the [newStartTime, newStopTime] in milliseconds

[mujz]

Let's rename clockTime to currentTime to be more precise and clear.

Suggested change

	const clockTime = new Date().getTime();
	const currentTime = new Date().getTime();

[mujz]

Can you spot the typo?

[mujz]

Better grammar

Suggested change

	* A function that take three parameters to clculate the time that appear on the UI
	* A function that takes three parameters to clculate the time that should appear on the UI

Also, can you spot the typo? (I didn't fix it, I want you to spot it and fix it).

[mujz]

Improve the documentation

Suggested change

	* A function that take two parameter {time2} is object have time as minute and seconds
	the second parameter {time1} is the time that subtracts, this function convert the
	the object to millisecond and calculate the deference between the two times return new
	time as minutes and seconds in an array
	* @param {number} time1 the time that subtracts
	* @param {object} time2 the time to subtract from
	* A function that calculates the deference between the two periods
	* @param {number} time1 the subtrahend period in milliseconds
	* @param {object} time2 the minuend period as an object that has "mins" and "secs" props
	* @returns {number} the duration in milliseconds between the two periods

Also, can you spot the typos?