-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add download demo #444
Add download demo #444
Conversation
src/ui/greeting/Greeting.scss
Outdated
margin-top: $spacer / 2; | ||
|
||
* { | ||
display: inline-block; |
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.
If you used a <span>
instead of a <p>
I don't think you need this block. You could simply move the font-size to &__DownloadDemo.
src/ui/greeting/Greeting.scss
Outdated
&__Link { | ||
color: $brand-primary; | ||
cursor: pointer; | ||
text-decoration: underline; |
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.
Perhaps we should move this to a more global place like styles/index.scss
?
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.
@kraenhansen I agree with this, maybe should we create a new card for unifying all the links styles?
@@ -20,4 +20,10 @@ body, | |||
height: 100%; | |||
} | |||
|
|||
.Link { |
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.
Was thinking more just an a
selector. Do you see an issue in that?
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.
@kraenhansen I prefer classes over tag selectors maybe we want to apply this styles to a button. Also, we will affect all the a
in the app, are you sure about that?
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 guess your right - a Link class is probably best.
closes #315