-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: display application period closed #62
Conversation
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.
Currently it works as a minimal solution, which is great 🚀
There are some minor improvements (that require some frontend date-logic) that I would like to see implemented. Regarding the visual representation I am not feeling too inspired at the moment. Perhaps @HenrikVL has some ideas?
@@ -1,7 +1,9 @@ | |||
import { Box, Button, createStyles } from '@mantine/core' | |||
import { Box, Button, createStyles, Loader } from '@mantine/core' |
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.
I would like this to inform the user a bit more:
- If the admission period is underway: "Frist for innsending av søknad DD-MM-YYYY 23.59"
- If the admission period has passed: "Opptaket var fra DD-MM-YYYY til DD-MM-YYYY."
- If the admission period is in the future: "Opptaket er fra DD-MM-YYYY til DD-MM-YYYY."
- If the admission period does not exist in db: "Opptaksperioden er ikke satt enda."
Not all cases are required. I find the first one the most important.
@Xtrah probably has some opinions on this 👂🏼
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.
I think the first one is definitely the most important ℹ️ It can even be simpler - "Søknadsfrist DD-MM-YYYY 23:59"
I don't think we need the other information. We could edit the general closedText
to something like this instead:
NTNUI Admin har for tiden ingen opptak
Vi har vanligvis opptak på starten av hvert semester.
Leter du etter opptak til en NTNUI gruppe eller lag? Sjekk gruppens egen nettside!
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.
@supapesh perhaps you could take a look at implementing this? I believe the suggestion Mats gave is fitting. ✨
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.
3b5c55e
to
e3e54a4
Compare
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.
Great job! I approve 🙌
dd251e1
to
c130adf
Compare
Closes #40
Summary of changes
/applications/period/active
) and assigns this to a state