Skip to content
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

Change font-sizes and adjust common layout to mobile #378

Merged
merged 11 commits into from
May 19, 2024
Merged

Conversation

Matushl
Copy link
Member

@Matushl Matushl commented May 19, 2024

cast riesenia #177, ale bude este potreba dalsie zmeny (mozno rozparsujem na mensie tasky)

komentare ku par commitom:

  • Try set some reasonable font-sizes: Dorot sa vyjadrila, ze to malo byt mensie ked to videla na obrazovke, tak som to pre md a lg proste zmensil nejak pomerovo
  • Adjust Hamburger size: uprava velkosti aby neblikal/nemenil topbar velkost pri otvarani menu
  • Change mobile sizes according to figma: nejaky prvy nastrel nech to je aspon trochu pozeratelne, kym sa prejdu jednotlive stranky a upravia jedna po druhej

@Matushl Matushl requested a review from rtrembecky May 19, 2024 08:11
@Matushl Matushl self-assigned this May 19, 2024
src/theme.ts Outdated Show resolved Hide resolved
src/theme.ts Outdated Show resolved Hide resolved
src/theme.ts Outdated
Comment on lines 201 to 202
fontSize: pxToRem(32),
[sm]: {fontSize: pxToRem(32)},
Copy link
Collaborator

@rtrembecky rtrembecky May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xs/sm teda ako su, dokonca vacsie ako md? ci to je, ze uz toho bolo moc a je to na follow-up?
EDIT: aha to su jedine male fonty, ktore si nezmenil. tak mozno si zabudol. kazdopadne, ak vyzeraju dobre, aj tak by som siel na 26 aby to bolo konzistentne s md

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no nezabudol, H1 je pouzite len pre nadpis v topbare, kde to vo Figme pre mobil je naozaj take obrovske

akoze mozeme ignorovat ten mobilny design "uplne", nvm co myslis?

Copy link
Member Author

@Matushl Matushl May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre porovnanie
image
image

Comment on lines +281 to +283
fontSize: pxToRem(11),
[sm]: {fontSize: pxToRem(11)},
[md]: {fontSize: pxToRem(9)},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zvlastne vidiet 11/11/9 ale asi ok 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cele tie font size su take divne, tiez ma boleli prsty ked som to pisal

@@ -70,9 +70,9 @@ export const SemesterPicker: FC<{page: 'zadania' | 'vysledky' | 'admin/opravovan
}

return (
<div className={styles.menu}>
<Stack sx={{flexDirection: 'row', alignItems: 'center', gap: 2, zIndex: 1000, userSelect: 'none'}}>
Copy link
Collaborator

@rtrembecky rtrembecky May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idealne Stack direction="row" a nesahat na flexDirection priamo.
EDIT: ale doesn't matter, asi sa na to este pozriem potom, ked chcem prerobit SemesterPicker

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ja som akosi vsetko natrepal do sx, lebo som mal pocit ze to tak chceme :-D

Copy link
Collaborator

@rtrembecky rtrembecky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job

@Matushl Matushl merged commit bfb5e03 into master May 19, 2024
1 check passed
@Matushl Matushl deleted the layout/mobile branch May 19, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants