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
Sash/update home page #26
Conversation
@@ -11,6 +11,13 @@ export const Text = styled.p` | |||
|
|||
/* prettier-ignore */ | |||
color: var(--color-${props => props.color || 'black'}); | |||
font-size: ${props => { |
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.
we can remove these, since we can use header component instead
src/themes/device.js
Outdated
@@ -8,6 +8,13 @@ const size = { | |||
desktop: '1980px', | |||
} | |||
|
|||
const breakpoint = { |
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.
size and breakpoint is the same thing actually, if you want to add more breakpoints, you can add it inside size
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 know, just wanted to check if we can simplify this and added that as example.
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.
ya, I think its good as well to simplify it. @OskarAhl
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.
@sash-binary can you simplify it in your PR?, remove sizes, and we can use breakpoints instead
src/pages/index.js
Outdated
* { | ||
max-width: 100%; | ||
} | ||
.card-title { |
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.
may I know why are we using className here?
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.
yeah please use props / check out how to nest styled components (instead of classNames)
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.
this was error, cleaned up
src/pages/index.js
Outdated
<Hero /> | ||
<DtraderSection> | ||
<SectionHeader> | ||
<h2><Logo /> {localize('DTrader')}</h2> |
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.
<Header />
?
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.
coming next
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.
can you please add description in the PR, so we will know what are the changes 🙏
…ible-ts-errors Niloofar / Bug - Fix Typescript errors in responsible
Add:
Update: