-
Notifications
You must be signed in to change notification settings - Fork 25
liad calculator #4
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "liveServer.settings.port": 5501 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| let firstNum = ''; | ||
| let secondNum = ''; | ||
| let operator = null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need to initialize this as null? the default undefined wont be good here? |
||
| let display = document.querySelector('.display'); | ||
|
|
||
| let numbers = document.querySelectorAll('.number'); | ||
|
|
||
| let plus = document.querySelector('#add'); | ||
| let sub = document.querySelector('#subtract'); | ||
| let mul = document.querySelector('#multiply'); | ||
| let div = document.querySelector('#divide'); | ||
|
|
||
| let equal = document.querySelector('#equal'); | ||
| let clear_page = document.querySelector('#clear') | ||
|
|
||
|
|
||
| numbers.forEach(number => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good use in the loop here |
||
| number.addEventListener('click', function(event) { | ||
| const value = event.target.textContent; | ||
|
|
||
| if (!operator) { | ||
| firstNum += value; | ||
| display.textContent = firstNum; | ||
| } else { | ||
| secondNum += value; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With early return you can make this shorter : if (!operator) {
firstNum += value;
display.textContent = firstNum;
return
secondNum += value;
display.textContent = secondNum;You will have to use another type of loop tho because you cannot exit the loop iteration when using |
||
| display.textContent = secondNum; | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| plus.addEventListener('click', function() { | ||
| operator = '+'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think that you can combine those 4 functions into a single one? they are kind of the same |
||
| display.textContent = operator; | ||
| }); | ||
|
|
||
| div.addEventListener('click', function() { | ||
| operator = '/'; | ||
| display.textContent = operator; | ||
| }); | ||
|
|
||
| sub.addEventListener('click', function() { | ||
| operator = '-'; | ||
| display.textContent = operator; | ||
| }); | ||
|
|
||
| mul.addEventListener('click', function() { | ||
| operator = '*'; | ||
| display.textContent = operator; | ||
| }); | ||
|
|
||
| equal.addEventListener('click', function() { | ||
| let result; | ||
| if (operator === '+') { | ||
| result = Number(firstNum) + Number(secondNum); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| if(operator === '-') { | ||
| result = Number(firstNum) - Number(secondNum); | ||
| } | ||
| if(operator === '/') { | ||
| result = Number(firstNum) / Number(secondNum); | ||
| } | ||
| if(operator === '*') { | ||
| result = Number(firstNum) * Number(secondNum); | ||
| } | ||
| display.textContent = result; | ||
|
|
||
| firstNum = result.toString(); | ||
| secondNum = ''; | ||
| operator = null; | ||
| }); | ||
|
|
||
| clear_page.addEventListener('click', function(){ | ||
| firstNum = ''; | ||
| secondNum = ''; | ||
| operator = null; | ||
| display.textContent = ''; | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,108 +1,62 @@ | ||
| :root { | ||
| --bg-color: #f3f4f6; | ||
| --calc-bg: #ffffff; | ||
| --btn-bg: #e5e7eb; | ||
| --btn-hover: #d1d5db; | ||
| --op-bg: #3b82f6; | ||
| --op-color: #ffffff; | ||
| --display-bg: #1f2937; | ||
| --display-color: #ffffff; | ||
| --text-color: #111827; | ||
| body { | ||
| position: relative; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this helping you ?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really helpful and thank you very much
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to know why you chose to do so, is it because of you wanted the children be absolute? something else? |
||
| } | ||
|
|
||
| * { | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| padding: 0; | ||
| font-family: 'Inter', system-ui, -apple-system, sans-serif; | ||
| .calc-title { | ||
| text-align: center; | ||
| font-size: 2.1rem; | ||
| font-weight: 700; | ||
| margin-top: 30px; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try not to mix different units, stick to one |
||
| margin-bottom: 25px; | ||
| color: #333; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Colors should come from css variables |
||
| padding-bottom: 8px; | ||
| border-bottom: 3px solid #ffbf69; | ||
| width: 100%; | ||
| } | ||
|
|
||
| body { | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| min-height: 100vh; | ||
| background-color: var(--bg-color); | ||
| } | ||
|
|
||
| .calculator { | ||
| background-color: var(--calc-bg); | ||
| padding: 20px; | ||
| border-radius: 20px; | ||
| box-shadow: 0 10px 25px rgba(0, 0, 0, 0.1); | ||
| width: 320px; | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 15px; | ||
| .container { | ||
| position: fixed; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think you can do the same with flex?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. of course tust me...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good, flex will be much better and modern approach |
||
| top: 50%; | ||
| left: 50%; | ||
| translate: -50% -50%; | ||
| } | ||
|
|
||
| .display { | ||
| background-color: var(--display-bg); | ||
| color: var(--display-color); | ||
| font-size: 2.5rem; | ||
| padding: 20px; | ||
| border-radius: 10px; | ||
| text-align: right; | ||
| width: 300px; | ||
| height: 60px; | ||
| border: 0.5px solid black; | ||
| border-radius: 5px; | ||
| margin-bottom: 10px; | ||
| overflow-x: auto; | ||
| } | ||
|
|
||
| .buttons { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 10px; | ||
| } | ||
|
|
||
| .row { | ||
| display: flex; | ||
| gap: 10px; | ||
| justify-content: space-between; | ||
| text-align: right; | ||
| font-size: 24px; | ||
| padding: 5px; | ||
| background-color: rgb(255, 249, 242); | ||
| } | ||
|
|
||
| button { | ||
| flex: 1; | ||
| padding: 15px; | ||
| font-size: 1.25rem; | ||
| border: none; | ||
| border-radius: 10px; | ||
| background-color: var(--btn-bg); | ||
| color: var(--text-color); | ||
| width: 70px; | ||
| height: 70px; | ||
| margin: 3px; | ||
| font-size: 20px; | ||
| border-radius: 5px; | ||
| border: 0.5px solid black; | ||
| cursor: pointer; | ||
| transition: all 0.2s ease; | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| button:hover { | ||
| background-color: var(--btn-hover); | ||
| transform: translateY(-2px); | ||
| } | ||
|
|
||
| button:active { | ||
| transform: translateY(0); | ||
| } | ||
|
|
||
| /* Operations Row Styling */ | ||
| .operations button { | ||
| background-color: var(--op-bg); | ||
| color: var(--op-color); | ||
| font-weight: bold; | ||
| .container button:nth-of-type(odd) { | ||
| background-color: blanchedalmond; | ||
| } | ||
|
|
||
| .operations button:hover { | ||
| filter: brightness(110%); | ||
| .container button:nth-of-type(even) { | ||
| background-color: rgb(255, 247, 228); | ||
| } | ||
|
|
||
| /* Last Row Styling */ | ||
| .last-row button { | ||
| background-color: #10b981; /* Green for equals/action */ | ||
| color: white; | ||
| .container button:nth-of-type(odd):hover { | ||
| background-color: rgb(255, 220, 167); | ||
| } | ||
|
|
||
| .last-row button:first-child { | ||
| background-color: #ef4444; /* Red for Clear */ | ||
| } | ||
|
|
||
| .last-row button:nth-child(2) { | ||
| background-color: var(--btn-bg); | ||
| color: var(--text-color); | ||
| } | ||
| .container button:nth-of-type(even):hover { | ||
| background-color: rgb(252, 235, 196); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need id in here? you can use classes for everything