-
Notifications
You must be signed in to change notification settings - Fork 1
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
Exercise side #20
base: main
Are you sure you want to change the base?
Exercise side #20
Conversation
✅ Deploy Preview for pen-pals ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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 progress so far! Once you fix these things and add the placeholder for the graph box (and fix any linting errors that crop up), it should be good to go.
<h1> Type the correct numbers into the blanks below! </h1> | ||
<h1> A = (2,0) </h1> | ||
<h1> B = (-2,0) </h1> | ||
<h1> C = (-1,0) </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.
A few small things:
- Given the layout of the text in the designs, it makes more sense in my opinion to have the actual "exercises," i.e. the problems they have to solve, in their own
div
(perhaps with its own ID oflevel-exercise-prompts
or similar?). This would allow you to add the additional spacing between the actual heading of the exercise (the main prompt at the top) and the questions without adding some arbitrary spacing between text items 1 and 2. In addition, it would make it easier to dynamically render exercises, for when we do that in the future. - Generally it's bad practice to overuse headings, especially the
h1
element which is reserved for top-level headings and the like. I suggest instead that you usep
tags for all of these and style them as needed. - The letters A, B, and C should be italicized; I suggest wrapping them in a
span
element and giving thespan
the appropriate styling for italics (I think it's something like "font-style: italic"?).
} | ||
|
||
button { | ||
background-color: green; |
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.
Don't forget to use the correct shade of green that's in the colors.scss
file! You can see an example of how to use variables from that file by looking at line 9 of this file. Just check the colors.scss
file for the variable name and include it according to the syntax from line 9. Also, there's no need for a border on this button as it's all one color; the Figma doesn't include any border either.
button { | ||
background-color: green; | ||
border: 1px solid #e1e4e8; | ||
border-radius: 10px; |
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.
Similar to the comment on colors, this should be 12px, not 10px.
Summary
#4 added the level text to the Exercise Side