fribyte-code / friByte.capture-the-flag

friBytes own Capture The Flag platform 🚩
MIT License
0 stars 0 forks source link

New styling #46

Closed Celsiusss closed 8 months ago

Celsiusss commented 8 months ago

And implemented some cool things. Like category tabs. And responsive UI (mobile friendly).

nheek commented 8 months ago

Har ikke sett selve nettsiden enda. Men hvorfor ikke tailwind?

Celsiusss commented 8 months ago

Screenshots?

Generally I'm a fan of keeping things simple -> Using a simple style library like Tailwind and DaisyUI or others to quickly iterate and have ready to use components which works good and looks good without a lot of custom css.

2024-02-11T15:41:16,477052636+01:00

Using CSS rather than components from DaisyUI are much more flexible. I also got a few complaints that it was difficult to contribute due to the learning curve of Tailwind.

IMO, using CSS over a component library is keeping things simple.

nheek commented 8 months ago

If you say it so about the learning curve. I kinda agree if they've never touched CSS before.

But I personally think Tailwind saves so much time. Especially when adding new elements inside already existing elements (like in-between elements).

Arguably, learning/finding out which CSS element to change is harder than changing a line in Tailwind. (Without messing things up that is).

simsine commented 8 months ago

Why?

mathiash98 commented 8 months ago

Screenshots? Generally I'm a fan of keeping things simple -> Using a simple style library like Tailwind and DaisyUI or others to quickly iterate and have ready to use components which works good and looks good without a lot of custom css. Using CSS rather than components from DaisyUI are much more flexible. I also got a few complaints that it was difficult to contribute due to the learning curve of Tailwind.

IMO, using CSS over a component library is keeping things simple.

Screenshot for all pages that are affected would be nice

Fair enough...

I would say it comes down to what we want to do:

Writing our own CSS-"component library" is a real time stealer and we'll probably just end up installing a React component library. -> Back to "not flexible"

Main benefit of tailwind + DaisyUI in my eyes:

polsevev commented 8 months ago

I think using something simple and maintainable is a better solution than Tailwind, more documentation, better practice for real life large application development, and easier use of 3rd party components (like datepickers). I support this!

simsine commented 8 months ago

Screenshots? Generally I'm a fan of keeping things simple -> Using a simple style library like Tailwind and DaisyUI or others to quickly iterate and have ready to use components which works good and looks good without a lot of custom css.

2024-02-11T15:41:16,477052636+01:00

Using CSS rather than components from DaisyUI are much more flexible. I also got a few complaints that it was difficult to contribute due to the learning curve of Tailwind.

IMO, using CSS over a component library is keeping things simple.

Learning curve of Tailwind? IMO tailwind lowers the barrier of entry here rather than raies it. While i do agree that writing our own CSS is more flexible this makes the code a lot messier and makes it way harder to maintain. This change really doesn't fix any of the existing UX problems and only serves to make them harder to fix.

mathiash98 commented 8 months ago

I think using something simple and maintainable is a better solution than Tailwind, more documentation, better practice for real life large application development, and easier use of 3rd party components (like datepickers). I support this!

3rd party components like datepickers should not be affected by what css styles (custom css or tailwind) we have? I would also argue that using css library or a normal component library is more relevant for developer work life

polsevev commented 8 months ago

Screenshots? Generally I'm a fan of keeping things simple -> Using a simple style library like Tailwind and DaisyUI or others to quickly iterate and have ready to use components which works good and looks good without a lot of custom css.

2024-02-11T15:41:16,477052636+01:00 Using CSS rather than components from DaisyUI are much more flexible. I also got a few complaints that it was difficult to contribute due to the learning curve of Tailwind. IMO, using CSS over a component library is keeping things simple.

Learning curve of Tailwind? IMO tailwind lowers the barrier of entry here rather than raies it. While i do agree that writing our own CSS is more flexible this makes the code a lot messier and makes it way harder to maintain. This change really doesn't fix any of the existing UX problems and only serves to make them harder to fix.

Hard disagree, using just CSS allows for anyone with any knowledge of web-dev to work on the project, relying on something like Tailwind or other component libraries will limit the number of people who can simply come in and contribute.

Also the issue about "messier" is only applicable if we allow messy code in the project, which would be the same problem with Tailwind or any other

polsevev commented 8 months ago

I think using something simple and maintainable is a better solution than Tailwind, more documentation, better practice for real life large application development, and easier use of 3rd party components (like datepickers). I support this!

3rd party components like datepickers should not be affected by what css styles (custom css or tailwind) we have? I would also argue that using css library or a normal component library is more relevant for developer work life

Say that to all the datepickers i tried to use last dugnad :)

nheek commented 8 months ago

Ig we're all speaking from experience.

But if I had to choose, vanilla CSS is surely more flexible.

But Tailwind provides easier application. I've taught someone who didn't have CSS knowledge Tailwind first (while also teaching the corresponding vanillas) and it went well.

nheek commented 8 months ago

I personally so far anyways, have not have problems when it comes to for example date pickers. I just use Tailwind-based components like NextUI and it works fine every time.

polsevev commented 8 months ago

Slik jeg ser det, har denne PRen mer enn bare "dropp tailwind", selve UIet er finere, mer responsivt og støtter mobil. Samt en finere implementasjon av Kategorier. Jeg synes det er nok til at vi bør vurdere å merge :)

Celsiusss commented 8 months ago

This change really doesn't fix any of the existing UX problems and only serves to make them harder to fix.

The goal was to make the styling more flexible to use and maintain.

This PR updates the color scheme and styling for dark and light mode. It also focuses on being responsive for all screen sizes.

Screenshot for all pages that are affected would be nice

Fair enough...

I would say it comes down to what we want to do:

* Writing CSS for all possible cases (hover, focus, keyboard focus, click)

* or use something that already is battle tested and is accessible.

Writing our own CSS-"component library" is a real time stealer and we'll probably just end up installing a React component library. -> Back to "not flexible"

Main benefit of tailwind + DaisyUI in my eyes:

* Looks pretty out of the box

* Out of the box support for themes

* Color themes is defined, easy to overwrite and get correct colors correct places

* Can install different Component libraries based on tailwind (lots of options)

* We still have access to writing custom CSS if we really want to

* Not being flexible is somewhat a good thing, user interfaces with lots of different styles is a pain to maintain over time

Seeing as the CTF site is being used by multiple organizations and is being developed and maintained more and more, I see this change as an investment for the future of the application.

By using CSS modules, we keep the stylings separate from the HTML. CSS has many extremely good learning resources online. By using Tailwind, you still need to know CSS to know what the different tailwind classes does.

mathiash98 commented 8 months ago

Slik jeg ser det, har denne PRen mer enn bare "dropp tailwind", selve UIet er finere, mer responsivt og støtter mobil. Samt en finere implementasjon av Kategorier. Jeg synes det er nok til at vi bør vurdere å merge :)

Personlig er eg ikke fan av fargepaletten og designendringene. og mangler litt spacing rundt om kring.

Kategoriselectoren er en bitteliten change som vi sikkert burde gjort utenfor denne PRen.

Ja pure CSS gir fordeler iform av at det er mer basic knowledge. Men da må ihvertfall sass fjernes. Synes sass er en uting egentlig.

Ser helt ok ut og kan være med på å merge så lenge sass blir fjernet som dependency.

simsine commented 8 months ago

Slik jeg ser det, har denne PRen mer enn bare "dropp tailwind", selve UIet er finere, mer responsivt og støtter mobil. Samt en finere implementasjon av Kategorier. Jeg synes det er nok til at vi bør vurdere å merge :)

Personlig er eg ikke fan av fargepaletten og mangler litt spacing rundt om kring.

Kategoriselectoren er en bitteliten change som vi sikkert burde gjort utenfor denne PRen.

Ja pure CSS gir fordeler iform av at det er mer basic knowledge. Men da må ihvertfall sass fjernes. Synes sass er en uting egentlig.

Ser helt ok ut og kan være med på å merge så lenge sass blir fjernet som dependency.

Det han sa :pray: For min del har det ikke noe å si om jeg skriver tailwind klassenavn eller lager mine egne, men jeg mener denne måten å gjøre det på med scss og ikke minst måten PR'en blir fremstilt er helt feil.

Celsiusss commented 8 months ago

Synes sass er en uting egentlig.

Hvorfor det? Sass sin nesting har jo igrunnen blitt standard, som du nevnte selv i CSS. Samme nesting blir brukt av biblioteker som styled-components.

nheek commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho).

But ig whatever the majority says 🙏

Celsiusss commented 8 months ago

For min del har det ikke noe å si om jeg skriver tailwind klassenavn eller lager mine egne, men jeg mener denne måten å gjøre det på med scss og ikke minst måten PR'en blir fremstilt er helt feil.

Du har rett i at jeg kunne laget PRen litt bedre, my bad.

mathiash98 commented 8 months ago

Synes sass er en uting egentlig.

Hvorfor det? Sass sin nesting har jo igrunnen blitt standard, som du nevnte selv i CSS. Samme nesting blir brukt av biblioteker som styled-components.

Å gå fra et "native css bibliotek" til et custom css rammeverk er et tilbakesteg når vi snakker om at det i lærer og gjør skal være brukbart mange steder over tid. Ja de aller fleste steder støtter sass om man aktiverer det, men man ender opp med å lære ting som ikke kan brukes alle steder.

simsine commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho).

But ig whatever the majority says 🙏

The UI still needs a lot of redesign because as it stands the UX is pretty bad especially on the admin panel. This PR only goes to change the way we would implement these changes and fixes a small ammount of what is still needed to be done.

polsevev commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho). But ig whatever the majority says 🙏

The UI still needs a lot of redesign because as it stands the UX is pretty bad especially on the admin panel. This PR only goes to change the way we would implement these changes and fixes a small ammount of what is still needed to be done.

Men vi rekker jo ikke gjøre noe mer før fristen som er i morgen? Så skjønner ikke helt hvorfor vi ikke godtar litt istedenfor ingenting

Celsiusss commented 8 months ago

Synes sass er en uting egentlig.

Hvorfor det? Sass sin nesting har jo igrunnen blitt standard, som du nevnte selv i CSS. Samme nesting blir brukt av biblioteker som styled-components.

Å gå fra et "native css bibliotek" til et custom css rammeverk er et tilbakesteg når vi snakker om at det i lærer og gjør skal være brukbart mange steder over tid. Ja de aller fleste steder støtter sass om man aktiverer det, men man ender opp med å lære ting som ikke kan brukes alle steder.

Okei, men du støtter å bruke native CSS nesting, selv med 70% browser support?

nheek commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho).

But ig whatever the majority says 🙏

The UI still needs a lot of redesign because as it stands the UX is pretty bad especially on the admin panel. This PR only goes to change the way we would implement these changes and fixes a small ammount of what is still needed to be done.

Men vi rekker jo ikke gjøre noe mer før fristen som er i morgen? Så skjønner ikke helt hvorfor vi ikke godtar litt istedenfor ingenting

Exactly why I haven't done it "yet" because the deadline falls short of my current schedule.

simsine commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho).

But ig whatever the majority says 🙏

The UI still needs a lot of redesign because as it stands the UX is pretty bad especially on the admin panel. This PR only goes to change the way we would implement these changes and fixes a small ammount of what is still needed to be done.

Men vi rekker jo ikke gjøre noe mer før fristen som er i morgen? Så skjønner ikke helt hvorfor vi ikke godtar litt istedenfor ingenting

IMO er denne PR'en sånn det står til nå en liten nedgradering fra det nåværende utseende. Jeg sier ikke at det ikke vil bli bedre om vi faktisk gjør litt endringer her men det er ikke bra nok, spessielt nå rett før fristen å gjøre en så drastisk omstrukturering/redesign. Veldig trojan horse opplegg det her må jeg si

mathiash98 commented 8 months ago

Har en uke til på å fikse dill og dall I guess? Skal fortsatt flytte ting til rett domene osv. Hovedfunksjonene som ble etterspurt er vel på plass uten denne PRen?

polsevev commented 8 months ago

Was honestly and actually gonna redesign the site myself (or initiate it anyways) but ig it's done now (still haven't seen it tho).

But ig whatever the majority says 🙏

The UI still needs a lot of redesign because as it stands the UX is pretty bad especially on the admin panel. This PR only goes to change the way we would implement these changes and fixes a small ammount of what is still needed to be done.

Men vi rekker jo ikke gjøre noe mer før fristen som er i morgen? Så skjønner ikke helt hvorfor vi ikke godtar litt istedenfor ingenting

IMO er denne PR'en sånn det står til nå en liten nedgradering fra det nåværende utseende. Jeg sier ikke at det ikke vil bli bedre om vi faktisk gjør litt endringer her men det er ikke bra nok, spessielt nå rett før fristen å gjøre en så drastisk omstrukturering/redesign. Veldig trojan horse opplegg det her må jeg si

Uenig i at dette er en nedgradering fra nåværende utseende altså. Men vi merger ikke om ikke alle er on board

polsevev commented 8 months ago

Har en uke til på å fikse dill og dall I guess? Skal fortsatt flytte ting til rett domene osv. Hovedfunksjonene som ble etterspurt er vel på plass uten denne PRen?

Joda, siste endringen har jeg i PR nå

mathiash98 commented 8 months ago

Okei, men du støtter å bruke native CSS nesting, selv med 70% browser support?

Er vel i grunn ingen som bruker de nettleserne som ikke støtter det? Husker ikke hva vi kompilerer nettsiden vår til, men den funker ikke på safari for øyeblikket for eksempel.

Personlig ville eg bare ikke brukt css nesting. I min erfaring er ikke det noe særlig utbredt enda

Celsiusss commented 8 months ago

men det er ikke bra nok

Hva mangler?

Okei, men du støtter å bruke native CSS nesting, selv med 70% browser support?

Er vel i grunn ingen som bruker de nettleserne som ikke støtter det? Husker ikke hva vi kompilerer nettsiden vår til, men den funker ikke på safari for øyeblikket for eksempel.

Personlig ville eg bare ikke brukt css nesting. I min erfaring er ikke det noe særlig utbredt enda

Kan nok fjerne sass og beholde nesting, kan også eventuelt se på å fjerne nesting. Men etter min mening blir det mer clean med nesting.

Celsiusss commented 8 months ago

Fjerna sass @mathiash98 @simsine