zksharding fix #149
zksharding fix #149
Conversation
|
I didn't check styles, only logic. Because this is the work for the qa engineer, not developer |
| @@ -0,0 +1,12 @@ | |||
| @use '~styles/mixins' as *; | |||
There was a problem hiding this comment.
Looks like the almost similar with LeftColumn. Why not to just create Column component with left/right props? It would be easy to refactor in the future than when you have more components.
| @@ -23,37 +24,18 @@ | |||
|
|
|||
| @include mobile { | |||
| padding-left: size($spacing12); | |||
| padding-top: size($spacing24 - 8); | |||
There was a problem hiding this comment.
Well, i mean that $spacing24 - 8 is equal to 24 - 8, isn't it right?
There was a problem hiding this comment.
Yes, it is equal to 24 - 8
Judging by the design, the padding should be 24 pixels, but due to the difference in font drawing (Figma and browser), we reduce the paddings to leave dependence on the $spacing24 variable
There was a problem hiding this comment.
Does this issue relate to the particular font (in our case Helvetica Neue LT Pro) or it is something with drawing font in general? May i ask you to elaborate on this a bit more?
There was a problem hiding this comment.
in general, Figma draws the font a little differently, but in this case it is related to a certain font. Throughout the project, I adjusted the margins to the font so that it was pixel perfect
| @@ -9,6 +9,7 @@ | |||
| display: flex; | |||
| align-items: flex-start; | |||
| padding-bottom: size(91); | |||
| margin-top: size(-1); | |||
There was a problem hiding this comment.
looks hacky, but if there is no other workarounds, okay. Not very important.
| @@ -20,6 +20,23 @@ type SecureProps = { | |||
| const Secure = ({ className, data: { title, description, content, footer } }: SecureProps) => { | |||
| const { isMobile } = useViewport() | |||
|
|
|||
| const columnContent = useMemo( | |||
There was a problem hiding this comment.
This page has no reasons to update during runtime in the browser, so useMemo makes no sense.
site/global.d.ts
Outdated
| @@ -0,0 +1,5 @@ | |||
| declare global { | |||
There was a problem hiding this comment.
This looks like overhead. Consider to just add a type in the component module, like
type Content = {
title: string
description: string
icon: string
}
...
content.reduce<[Array<Content>, Array<Content>]>
No description provided.