Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review #9

Open
wants to merge 91 commits into
base: master
Choose a base branch
from
Open

Review #9

wants to merge 91 commits into from

Conversation

QaysAyad
Copy link

No description provided.

hanan_shirin_qays/moivesapp/package.json Outdated Show resolved Hide resolved
hanan_shirin_qays/moivesapp/src/App.js Outdated Show resolved Hide resolved
function Footer() {
return (
<div className="main-footer">
<small style={{ marginRight: "20px" }}>Developed by </small>-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use inline styles, and have it in external css file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one we removed it all but still we had used some inline styling along the project it is not worth it to make a class for it it is so specific for one component

hanan_shirin_qays/moivesapp/src/Components/Navbar.js Outdated Show resolved Hide resolved
import DropdownCategories from "./DropdownCategories";
import { constructUrl } from "./Api";

export default function SearchBox(props) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract the props here too

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what you mean ?

hanan_shirin_qays/moivesapp/src/Components/SearchBox.js Outdated Show resolved Hide resolved
hanan_shirin_qays/moivesapp/src/index.css Outdated Show resolved Hide resolved
handleQuery={props.handleQuery}
handleMovies={props.handleMovies}
isLoading={props.isLoading}
setIsLoading={props.setIsLoading}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you passing this here?

@osamaakb
Copy link
Contributor

osamaakb commented Jul 1, 2020

really good job, like how you separated the concerns, can you please put the github pages URL here too.
also left you some comments make sure to check!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants