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

Legger til støtte for nye læringssti steg. #2323

Merged
merged 9 commits into from
Feb 3, 2025
Merged

Conversation

MaPoKen
Copy link
Contributor

@MaPoKen MaPoKen commented Jan 23, 2025

depends on NDLANO/graphql-api#539

Legger til støtte for nye typer læringssti steg og splitter opp renderingen i flere filer for å letter se forskjellen.

@MaPoKen MaPoKen requested a review from a team January 23, 2025 08:46
@MaPoKen MaPoKen marked this pull request as ready for review January 23, 2025 10:48
Copy link
Member

@gunnarvelle gunnarvelle left a comment

Choose a reason for hiding this comment

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

lgtm

@MaPoKen MaPoKen force-pushed the split-learningpathsteps branch from 46c3e71 to 2b6cf4d Compare January 30, 2025 11:48
@MaPoKen MaPoKen force-pushed the split-learningpathsteps branch from 2b6cf4d to 23d0ac2 Compare January 30, 2025 11:55
src/components/Learningpath/LearningpathMenu.tsx Outdated Show resolved Hide resolved
Comment on lines +95 to +96
const learningpathStepResource = learningpathStep.resource ?? data;
const resource = learningpathStep.resource ?? data?.node;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ser det er en del håndtering av at learningPathStep.resource kan være undefined i denne filen, men det kan vel fjernes ettersom du allerede sjekker for undefined i LearningpathStep? eller er det stress å få til med gql typene?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Det er litt stress samt. dette er egentlig bare kopiert over fra den svære LearningpathEmbed fila. Litt redd for å tulle med noe 🤠

@MaPoKen MaPoKen force-pushed the split-learningpathsteps branch from 1c73441 to de47ee3 Compare January 31, 2025 11:24
@MaPoKen MaPoKen force-pushed the split-learningpathsteps branch from fbbefa1 to 1c989d1 Compare February 3, 2025 07:56
@katrinewi
Copy link
Contributor

Ikke direkte relatert til denne, så det kan sikkert gjøres i egen PR, men vi burde kanskje sjekke at url er på riktig format før vi sender av gårde requesten i formen for å legge til innhold fra et annet nettsted? Får veldig mange "Invalid URL"- feil fra gql hvis man prøver å skrive noe i input-feltet

@katrinewi
Copy link
Contributor

En annen litt rar greie som heller ikke er direkte relatert til denne: hvis man legger til artikkel fra NDLA for så å slette den igjen får man mulighet til å lagre steget, det burde vel være disablet!

@katrinewi
Copy link
Contributor

katrinewi commented Feb 3, 2025

Skjermbilde 2025-02-03 kl  09 10 32

her: /minndla/learningpaths/3400/edit/steps

@MaPoKen MaPoKen merged commit 0020909 into master Feb 3, 2025
6 checks passed
@MaPoKen MaPoKen deleted the split-learningpathsteps branch February 3, 2025 09:07
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.

3 participants