-
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
feature/hexcode-support #29
Conversation
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.
The new comments and hex code algorithm looks really good, however, grid doesn't seem to be working quite right. There was also a small typo in the JS file.
@@ -113,17 +112,39 @@ input:focus { | |||
border-radius: 25% 15%; | |||
} | |||
|
|||
.cards { |
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.
Grid isn't looking so good on mobile...
And things aren't quite right on larger screens either...
I think that the margins and padding are messing up centering or something, but I haven't really had time to learn grid very well yet, so I don't really know. Maybe it would be better to just use flexbox?
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.
Updated with some changes that should fix these. However, I don't see this as too big of a concern if a modal feature will be implemented to display rgb and hex values. With the modal the card will only be the box with a filled in color and will fit perfectly into whichever layout scheme we go with.
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.
True, these changes won't make much difference in the long run, but for now this looks much better. Thanks!
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.
Changes look good!
@@ -113,17 +112,39 @@ input:focus { | |||
border-radius: 25% 15%; | |||
} | |||
|
|||
.cards { |
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.
True, these changes won't make much difference in the long run, but for now this looks much better. Thanks!
closes #21