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

chore: add optional courses #63

Merged
merged 7 commits into from
Oct 4, 2023
Merged

chore: add optional courses #63

merged 7 commits into from
Oct 4, 2023

Conversation

kocierik
Copy link
Member

@kocierik kocierik commented Sep 19, 2023

Da approvare dopo:

ho aggiunto anche la ui dei corsi a scelta copiando semplicemente quella dei corsi obbligatori

@vercel
Copy link

vercel bot commented Sep 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dynamik ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 11:26am

@kocierik kocierik changed the title chore: update type chore: add optional courses Sep 19, 2023
@foxyseta foxyseta linked an issue Sep 19, 2023 that may be closed by this pull request
Copy link
Member

@foxyseta foxyseta left a comment

Choose a reason for hiding this comment

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

In page.svelte vedo in effetti un po' troppo copia e incolla, ma non so nulla di Svelte quindi non so se ci siano ragioni per non rendere il codice modulare in questo caso.

Approvo l'idea di forzare i corsi a scelta dopo quelli obbligatori all'interno dello stesso anno, anche se il file non li dovesse avere in fondo in futuro.

@foxyseta foxyseta added the enhancement New feature or request label Sep 19, 2023
@kocierik
Copy link
Member Author

In page.svelte vedo in effetti un po' troppo copia e incolla, ma non so nulla di Svelte quindi non so se ci siano ragioni per non rendere il codice modulare in questo caso.

Approvo l'idea di forzare i corsi a scelta dopo quelli obbligatori all'interno dello stesso anno, anche se il file non li dovesse avere in fondo in futuro.

anche a me non piace molto la struttura, se ci sono alternative sono favorevole

@foxyseta
Copy link
Member

Come per gli altri framework, ci sono i componenti innestati.

@foxyseta
Copy link
Member

Ora che cartabinaria/config#4, rimane solo da riscrivere il codice di questa PR in modo modulare e poi si può accettare.

@kocierik
Copy link
Member Author

kocierik commented Oct 1, 2023

Ho fatto il merge del main e aggiunto il componente innestato

è rimasto un errore nella variabile data con tipo PageData che non riesco a sistemare

@VaiTon
Copy link
Member

VaiTon commented Oct 1, 2023

@kocierik se possibile farei il filter in +page.svelte e passerei le due liste filtrate

@kocierik
Copy link
Member Author

kocierik commented Oct 3, 2023

dovrei aver fatto, se puo' andare mergio @csunibo/frontend

Copy link
Member

@VaiTon VaiTon left a comment

Choose a reason for hiding this comment

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

Un po' di suggerimenti :)

src/lib/teachings.ts Outdated Show resolved Hide resolved
src/routes/ListTeaching.svelte Outdated Show resolved Hide resolved
src/routes/ListTeaching.svelte Outdated Show resolved Hide resolved
src/routes/dash/[course]/+page.svelte Outdated Show resolved Hide resolved
src/routes/ListTeaching.svelte Outdated Show resolved Hide resolved
src/routes/dash/[course]/+page.svelte Outdated Show resolved Hide resolved
@VaiTon
Copy link
Member

VaiTon commented Oct 4, 2023

@kocierik rebase pls 🎉

@kocierik
Copy link
Member Author

kocierik commented Oct 4, 2023

@kocierik rebase pls 🎉

fatto fatto

Copy link
Member

@VaiTon VaiTon left a comment

Choose a reason for hiding this comment

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

Anche se la ricorsione mi fa veramente acido, per ora per me possiamo mergiare

@kocierik kocierik merged commit a177223 into main Oct 4, 2023
5 checks passed
@kocierik
Copy link
Member Author

kocierik commented Oct 4, 2023

Ho notato che nel deploy il filtro non funziona (vedi magistrale informatica) avete idee del perche'?

@VaiTon
Copy link
Member

VaiTon commented Oct 4, 2023

@kocierik
Copy link
Member Author

kocierik commented Oct 4, 2023

@kocierik https://dynamik-git-chore-update-type-csunibo.vercel.app/dash/informatica-magistrale

Non te lo mostra con "1 anno facoltativi"?

ops si, penso sia stato un problema di caching, c'è solo da sistemare la visualizzazione dell'icona per telegram perché da mobile è rotta ho notato

@VaiTon VaiTon deleted the chore/update-type branch October 25, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Corsi a scelta
3 participants