-
Notifications
You must be signed in to change notification settings - Fork 2
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
main css #39
base: main
Are you sure you want to change the base?
main css #39
Conversation
…w team. Minor css adjustments on the buttons in Welcome.css
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.
Nice! The white is more pleasant touch
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.
These changes make the app look really good. Super slick. (I'm a fan of transparency. 😄 )
It looks like there are a few different issues/components being addressed in one branch, which could get really confusing fast so make sure everyone's clear when you start to merge your branches.
But the solutions made for each issue/component are, as usual, as simple as possible and no simpler. Great job!
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'm glad to see the application coming together! I would recommend to stay consistent with the unit you use to set padding and margins. It seems that some rules use percentages while others use rems. Try to convert the % to rems. Using % makes sense for width and height properties, however.
} | ||
|
||
.not-so-soon { | ||
color: green; | ||
@media only screen and (min-width: 736px) { |
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.
Yay! Media queries.
background-color: rgba(0, 0, 0, 0.25); | ||
} | ||
/* | ||
NEEDS TO BE REMOVED TO ANOTHER CSS FILE. |
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.
There's nothing like today to get this done 🙂
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.
Just a head's up . . .
This gets handled in @lacrivella 's modal css branch.
.additem__form { | ||
width: 90%; | ||
margin: 0; | ||
padding: 5%; |
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.
Try to stick to one kind of unit. What would 5% be in rem?
…w team. Minor css adjustments on the buttons in Welcome.css
For an example of how to fill this template out, see this Pull Request.
Description
Added a div with a class name of
main
intoApp.js
to allow us to have the same background throughout our app. Changed it from black to whitesmoke/light gray based on our discussion as a team---basically switched our font and background div colors. Minor css edits on the button. Updatedh1
andh2
inWelcome.js
to now be ah1
with aspan
.Update passes Accessibility Insights extension.
Related Issue
Acceptance Criteria
Type of Changes
Updates
Before
After
Testing Steps / QA Criteria