feat/components/buttons: added primary, text and link buttons #20
Conversation
| @@ -7,6 +7,9 @@ | |||
| rel="icon" | |||
| href="https://emojipedia-us.s3.dualstack.us-west-1.amazonaws.com/thumbs/240/apple/271/nerd-face_1f913.png" | |||
| /> | |||
| <link href="https://fonts.googleapis.com/css2?family=Work+Sans:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap" | |||
There was a problem hiding this comment.
Fonts are really heavy in size. Could you import only the weights we're using?
| @@ -0,0 +1,5 @@ | |||
| import React from 'react'; | |||
|
|
|||
| export default function Logo() { | |||
There was a problem hiding this comment.
Could you change all
export default function MyFunction() { ... }
to
export const MyFunction = () => { ... }
From my experience named exports work much better than default ones. VSCode can import them as autosuggestions as you type.
Change from functions to anonymous functions (arrow functions) is also a big yes. Why? You're not using this and for the most part, you shouldn't use this - it leads to confusion. It's a rule of thumb: if you're not using this in your functions, use anonymous function (arrow function)
| import React from 'react'; | ||
|
|
||
| export default function Logo() { | ||
| return <h3><span style={{ color: '#F53314' }}>Trip</span><img alt="Atomic layout" src="./Vector.png" /><span style={{ color: '#9C1B07' }} >Planner</span></h3>; |
There was a problem hiding this comment.
alt should have relevant description of the image. Change it to TripPlanner, it's more accurate than Atomic Layout
There was a problem hiding this comment.
And looking from more holistic perspective: please use logo from Figma. Just export the whole logo, import it here and make sure it's scaled as in the designs.
Here's the guide to exports in Figma: https://help.figma.com/hc/en-us/articles/360040028114-Guide-to-exports-in-Figma#h_41b394ed-f969-4840-9e6e-c7c2790d5bc5
| left: calc(50% - 105px/2); | ||
| top: calc(50% - 34px/2 + 54px); |
There was a problem hiding this comment.
It looks like you've probably copied every style from Figma. Copying every style from Figma doesn't work. Why?
Figma is just a showcase for our design system. Components have to be composable and reusable. If you specify, width, left and top, they will not look properly when using texts of different lengths, etc.
Please make sure that components are reusable, not tied up to specific use case.
| left: calc(50% - 105px/2); | ||
| top: calc(50% - 34px/2 + 54px); |
| cursor: pointer; | ||
| left: calc(50% - 105px/2); | ||
| top: calc(50% - 34px/2 + 54px); | ||
| font-family: Work Sans, sans-serif; |
There was a problem hiding this comment.
You can extract this style to global one, to not repeat yourself (keeping code DRY).
See src/GlobalStyle.ts
| const Nav = styled.ul` | ||
| width: 50%; | ||
| display: flex; | ||
| list-style-type: none; | ||
| justify-content: flex-end; | ||
| font-family: Work Sans, sans-serif; | ||
| font-style: normal; | ||
| font-weight: 500; | ||
| font-size: 13px; | ||
| line-height: 15px; | ||
| color: #919191; | ||
| & > li { | ||
| padding-right: 1rem; | ||
| } | ||
| `; | ||
|
|
||
| const NavBar = styled.div` | ||
| display: flex; | ||
| align-items: center; | ||
| width: 100%; | ||
| justify-content: space-between; | ||
| & > h3 { | ||
| padding-left: 1rem; | ||
| font-family: Work Sans, sans-serif; | ||
| font-style: normal; | ||
| font-weight: 600; | ||
| font-size: 17.4703px; | ||
| line-height: 20px; | ||
| letter-spacing: -0.03em; | ||
| } | ||
| `; | ||
|
|
There was a problem hiding this comment.
These components are not part of typography. Could you extract them to separate directories under src/components?
Also please do not style styled-components sass-style. If you have to refer to other component, please make yourself familiar with this: https://styled-components.com/docs/advanced#referring-to-other-components
|
Do you have prettier and eslint installed in VSCode? Please make sure you do, it'll be so much easier for writing cleaner-looking code Prettier: https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode |
|
@Pawel-dev5 any progress here? |
|
@marcinbar7 I made all corrections with Kacper's suggestion, waiting for approval |
| <script type="module" src="/src/index.tsx"></script> | ||
| </body> | ||
|
|
||
| </html> |
There was a problem hiding this comment.
Please run prettier on this file
feat/components/buttons: added primary, text and link buttons