Skip to content

Calculator JS evening#6

Open
gee92 wants to merge 1 commit intoReturn-Ready-2021-JavaScript-Evening:mainfrom
gee92:main
Open

Calculator JS evening#6
gee92 wants to merge 1 commit intoReturn-Ready-2021-JavaScript-Evening:mainfrom
gee92:main

Conversation

@gee92
Copy link
Copy Markdown

@gee92 gee92 commented Nov 16, 2020

No description provided.

@kburd kburd self-requested a review November 17, 2020 16:09
Comment thread calc.css
background-color: grey;
}

.frow{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All of these row selectors have the same styling. Create one selector and use it throughout your HTML (-1)

Comment thread calc.css
font-size: 1em;
color: white ;
}
a:hover{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wasn't required but nice additional touch!

Comment thread scientific.html

<h1> Scientific | <a href="standard.html"> Standard</a> </h1>

<input type="text" class="Scientific" name="display">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inputs should always go in a form (-2)

Comment thread scientific.html
</div>

<div>
<button class="4row"> e</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't see the 4row styling (-1)

Comment thread scientific.html
</html>
</body>

</Html> No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor styling issues, make sure indentation is consistent (-1)


95% Nice work!

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