Skip to content

Rifaq whats app clone sidebar#8

Open
rifaq-ahmer wants to merge 4 commits intomasterfrom
rifaq-whatsApp-clone-sidebar
Open

Rifaq whats app clone sidebar#8
rifaq-ahmer wants to merge 4 commits intomasterfrom
rifaq-whatsApp-clone-sidebar

Conversation

@rifaq-ahmer
Copy link
Copy Markdown
Contributor

No description provided.

@rifaq-ahmer rifaq-ahmer self-assigned this Nov 1, 2021
@rifaq-ahmer rifaq-ahmer requested a review from a team November 1, 2021 17:06

const AddForm = () => {
const [categoryOpen, setCategoryOpen] = useState(false);
const cat = categories;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you need to assign to another const?

const [modalOpen, setModalOpen] = useState(false);
const handleTitle = (e) => {
setTitle(e.target.value);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

single line

setAmount("");
return;
}
setAmount(val);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you could use ternary ..
isNaN(val) ? setAmount(“”) : setAmount(val);

const handleSubmit = () => {
if (title === "" || amount === "" || !category) {
const notify = () => toast("Please enter complete data");
notify();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do u need notify const? You could directly call the toast method

<input
placeholder="Give a name to your expenditure"
value={title}
onChange={(e) => handleTitle(e)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
onChange={(e) => handleTitle(e)}
onChange={handleTitle}

<input
placeholder="Enter Amount"
className="amount-input"
onChange={(e) => handleAmount(e)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
onChange={(e) => handleAmount(e)}
onChange={handleAmount}

}

@media only screen and (max-width: 1024px) {
.card-title {
Copy link
Copy Markdown

@AlishaPrasad AlishaPrasad Nov 2, 2021

Choose a reason for hiding this comment

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

Suggested change
.card-title {
.card-title,
.card-amount {

}
.card-amount {
font-size: 18px;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove

}

@media only screen and (max-width: 512px) {
.card-title {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
.card-title {
.card-title,
.card-amount {

}
.card-amount {
font-size: 16px;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove


const Footer = () => {
return <footer>Made with ❤️ by Rifaq</footer>;
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Single line

type="text"
placeholder="Search for Expenses"
value={query}
onChange={(event) => handleQuery(event)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
onChange={(event) => handleQuery(event)}
onChange={handleQuery}

padding: 12px 0;
align-items: center;
}
.home-topfold {
Copy link
Copy Markdown

@AlishaPrasad AlishaPrasad Nov 2, 2021

Choose a reason for hiding this comment

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

Suggested change
.home-topfold {
.home-topfold,
.add-topfold {

display: flex;
justify-content: space-between;
flex: 1;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

remove after above change

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.

2 participants