Skip to content

Conversation

@OfekSagiv
Copy link

No description provided.

@GilHeller GilHeller self-requested a review December 7, 2024 17:11
Copy link
Contributor

@GilHeller GilHeller left a comment

Choose a reason for hiding this comment

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

Overall nice job!
please see my comments

Copy link
Contributor

Choose a reason for hiding this comment

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

add .idea/ to .gitignore

Copy link
Contributor

Choose a reason for hiding this comment

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

add .idea/ to .gitignore

Copy link
Contributor

Choose a reason for hiding this comment

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

add .idea/ to .gitignore

Copy link
Contributor

Choose a reason for hiding this comment

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

add .idea/ to .gitignore

Copy link
Contributor

Choose a reason for hiding this comment

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

add .idea/ to .gitignore

/ person
<div class="tip-amount-container">
<div class="flex-box-tip">
<p>Tip amount<br><span style="color:hsl(184, 14%, 56%)">/ person</span></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you use inline styling instead of a class and css?

This can cause a mixed style dependencies. pick on styling method and stick with it.


// הצגת התוצאה בתיבה
document.getElementById('total-amount-input').value = `$${result}`;
console.log("bill amount:", billAmount, "num of ppl:", howManyPpl, "tip:", tipPercentage, "result:", result); // הדפסת הערכים בקונסול
Copy link
Contributor

Choose a reason for hiding this comment

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

redundant

display: flex;
justify-content: center;
align-items: center;
background-color: hsl(185, 41%, 84%);
Copy link
Contributor

Choose a reason for hiding this comment

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

const totalWithTip = billAmount * (1 + tipPercentage);
const result = howManyPpl > 0 ? (totalWithTip / howManyPpl).toFixed(2) : 0;

// הצגת התוצאה בתיבה
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid writing comments in Hebrew.

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