-
Notifications
You must be signed in to change notification settings - Fork 15
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
Part two answer #13
base: master
Are you sure you want to change the base?
Part two answer #13
Conversation
…CSS styling, more reading needed.
|
||
<style> | ||
body { |
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.
Good css work here.
subtractNumbers: function () { | ||
|
||
let numberOne = parseInt(document.forms.inputs.numberOne.value); |
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.
This all looks solid.
index.html
Outdated
</script> | ||
|
||
</head> | ||
|
||
<body> | ||
<div>Header</div> | ||
<H1>Calculator 1.2</H1> |
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.
I like the versioning!
index.html
Outdated
</div> | ||
<div> | ||
<button type="button" class="btn btn-primary" onclick="calculator.addNumbers()" >Add</button> | ||
<button type="button" class="btn btn-primary">Substract</button> | ||
<button type="button" class="btn btn-primary" padding: 50px 25px |
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.
This all looks good. There is an issue though. The "padding" and related values all has to e removed. You can add that to the btn, btn-primary or other css class if you want to include padding. It is not valid to put a direct padding attribute on any html tag.
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.
Please try to resolve the use of the padding: 50px 25px
that is in the tags. This code belongs in the style tag associated to a class name selector.
Got it! Thanks for looking over all this, I'll fix the add logic/concat.
issues and incorrect inline button padding in the morning! I've been
digging into more css guides today, will be pushing up a few new things
too. -Nick.
…On Tue, Jun 16, 2020, 22:34 John Rice ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Please review the add logic related to strings and concatenation. Also
please try to resolve the use of the padding: 50px 25px that is in the
tags. This code belongs in the style tag associated to a class name
selector.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#13 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGFHTII2QJU4PN4VNPG3Y5DRXATTPANCNFSM4N4VZN7A>
.
|
…unctions work as expected.
Basic introduction of other operators and changing var usage to let. Want to edit the number entering form to be able to take any given numbers and introduce button for 'equals' operation in the near future.