-
Notifications
You must be signed in to change notification settings - Fork 5
Added event and header #9
base: master
Are you sure you want to change the base?
Conversation
src/components/Event.jsx
Outdated
padding="20px" | ||
marginBottom={events.length < index ? "20px" : ""} | ||
alignItems="center" | ||
backgroundColor={index % 2 === 0 ? "000000" : "simonyi"} |
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.
A "000000" előtt hiányzik egy "#", bár szerintem jóval olvashatóbb volna a beépített "black" szín használata.
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.
Illetve még jobb az index maradékának vizsgálata helyett a Chakra UI PseudoBox
komponensének használata.
src/components/Event.jsx
Outdated
padding="20px" | ||
marginBottom={events.length < index ? "20px" : ""} |
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.
Konkrét pixel értékek helyett javasolnám a beépített téma által kínált spacing skála használatát. Jelen esetben az 5
érték használata volna célravezető.
src/components/Event.jsx
Outdated
<Flex | ||
flexDirection="column" | ||
padding="20px" | ||
marginBottom={events.length < index ? "20px" : ""} |
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.
Ez az állítás sosem értékelődik ki igazra, mivel 0 <= index < events.length
.
alignItems="center" | ||
backgroundColor={index % 2 === 0 ? "000000" : "simonyi"} | ||
> | ||
<div style={{ maxWidth: "850px", width: "100%" }}> |
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.
A Chakra UI <Box maxWidth="4xl" width="100%">
(4xl = 56rem = 896px) komponensének használata szerencsésebb beépített stílusok helyett. Ez akár egy újrafelhasználható Container
komponensben is helyet kaphatna egy külön fájlban:
import { Box } from '@chakra-ui/core';
export function Container(props) {
return <Box maxWidth="4xl" width="100%" {...props} />;
}
> | ||
<div style={{ maxWidth: "850px", width: "100%" }}> | ||
<Heading | ||
color={index % 2 === 0 ? "simonyi" : "#fff"} |
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.
Itt is javasolnám inkább a "white" szín, valamint a PseudoBox
komponens használatát. Utóbbival JS helyett CSS-ben jelennek majd meg a stílusok, ami böngésző-kompatibilitás, strukturáltság és szempontjából egyaránt előnyös.
</Link> | ||
</div> | ||
<Menu> | ||
<MenuButton className="collapse-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.
Itt szintén nem érdemes a className
propot alkalmazni. Helyette inkább a Chakra UI propjaiból érdemes építkezni.
.display-header { | ||
display: initial; | ||
} | ||
.collapse-header { | ||
display: none; | ||
} | ||
@media screen and (min-width: 768px) { | ||
.aboutimage { | ||
clip-path: ellipse(50% 75% at 65% 50%); | ||
} | ||
} | ||
@media screen and (max-width: 536px) { | ||
.collapse-header { | ||
display: initial; | ||
} | ||
.display-header { | ||
display: none; | ||
} | ||
} |
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.
Bár ez a CSS fájl már eddig is a kódbázis része volt, szerintem célszerű volna megválni tőle, kizárólagosan a Chakra UI lehetőségeire támaszkodva.
@@ -0,0 +1,24 @@ | |||
[ | |||
{ | |||
"name": "Gólyakörte", |
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.
Ebben nem vagyok biztos, de szerintem itt a "GólyaKörTe" volna a helyes írásmód.
src/data/events.json
Outdated
"description": [ | ||
{ | ||
"title": "Kik vagyunk?", | ||
"text": ["A Simonyi Károly szakkollégium csapata!"] |
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.
"szakkollégium" -> "Szakkollégium"
{ | ||
"title": "Miért mi vagyunk a legjobbak?", | ||
"text": [ | ||
"Az egyetem mellett nálunk rengeteg emberrel megismerkedhetsz, és szerteágazó tudásra tehetsz szert", | ||
"Ha érdekelnek az olyan események, mint a Simonyi Konferencia, vagy csak simán szeretnél például forrasztani, fényképezni, programozni, vagy akár designer lenni, akkor itt a helyed." | ||
] | ||
}, |
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.
Remélem, a megfogalmazás még finomításra kerül majd 🙂
Added event and header