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

feat(edit): settings in one form #1823

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat(edit): settings in one form #1823

wants to merge 8 commits into from

Conversation

purusott
Copy link
Collaborator

@purusott purusott commented Feb 24, 2025

En lagreknapp for tavleinnstillinger


Motivasjon

Ny layout på edit siden

Endringer

  • Laget en ny komponent Settings som har alle innstillinger. Her skjer submit eventet med evt error handling, som sendes til hver komponent
  • En tilhørende server action som kjører tidligere actions i en
  • image

Sjekkliste for Review

  • Verifiser at knapper ikke henger
  • Sjekk om noe mangler i server action, og at data blir lagret riktig

@purusott purusott requested a review from emilielr as a code owner February 24, 2025 09:55
@@ -78,7 +78,7 @@ function CompressSurvey() {

return (
<>
<div className="box flex flex-col">
<div className="box flex flex-col w-1/2">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Synes denne ser litt rar ut 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤷‍♂️

Comment on lines +1 to +16
'use client'
import { Heading4 } from '@entur/typography'
import { TFontSize } from 'types/meta'
import { TBoardID } from 'types/settings'
import { FontChoiceChip } from './FontChoiceChip'

function FontSelect({ font }: { bid: TBoardID; font: TFontSize }) {
return (
<div>
<Heading4 margin="bottom">Tekststørrelse </Heading4>
<FontChoiceChip font={font} />
</div>
)
}

export { FontSelect }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Trenger denne å være en egen komponent lenger? Man kan vel bare ta Headingen inn i FontChoiceChip

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Da blir det rart i edit mappe, siden den har heading og paragraph utenfor nå:
image

Comment on lines 80 to 94
await firestore()
.collection(fromOrg ? 'organizations' : 'users')
.doc((fromOrg || user?.uid) ?? '')
.update({
[fromOrg ? 'boards' : 'owner']:
firestore.FieldValue.arrayRemove(bid),
})

await firestore()
.collection(personal || !organization ? 'users' : 'organizations')
.doc((personal || !organization ? user?.uid : organization) ?? '')
.update({
[personal || !organization ? 'owner' : 'boards']:
firestore.FieldValue.arrayUnion(bid),
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

kan dette gjøres mer lesbart? Kanskje verdt det å kjøre en if-løkke rundt for å sjekke om boardet tilhører organisasjon eller ikke

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gjenbruker nå tidligere moveboard funksjon

const errors = await saveForm(data, selectedPoint?.value as TLocation)

if (!errors) {
setFormErrors({})
Copy link
Collaborator

Choose a reason for hiding this comment

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

trengs denne? er ikke errors tom fra før av?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Måtte gjøre det for å håndtere når man allerede har en form error og fikser på de, ellers blir ikke tidligere feil borte

Comment on lines 29 to 30
const organization =
organizationString === 'undefined' ? undefined : organizationString
Copy link
Collaborator

Choose a reason for hiding this comment

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

hva skjer her? kanskje legg inni if-løkke for økt lesbarhet

value: data.get('viewType') as string,
})

const errors = await saveForm(data, selectedPoint?.value as TLocation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

hvorfor legges organization til med data.append, mens location sendes inn som eget argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fikset på det, org sendes inn som de andre innstillingene. Location må derimot sendes inn på denne måten pga nested object

@SelmaBergstrand
Copy link
Collaborator

Bug: å fjerne adresse-feltet fjerner ikke lenger gåavstand på tiles

onChange={() => setOverride(!override)}
name="override"
>
Vis organisasjons infomelding
Copy link
Collaborator

Choose a reason for hiding this comment

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

Vis infomelding fra organisasjonen.


return (
<div>
<Heading4 margin="bottom">Organisasjon</Heading4>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Obs: husk at i dev og prod nå er organisasjon byttet ut med tavler overalt, så vi må ikke introdusere "organisasjon" noe sted igjen

Comment on lines 21 to 24
<Heading4 margin="bottom">Gangavstand</Heading4>
<Paragraph className="mb-2">
Vis gåavstanden fra tavlens adresse til stoppestedet.
</Paragraph>
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Gangavstand" begge steder, ikke gåavstand

Copy link
Collaborator

Choose a reason for hiding this comment

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

og denne teksten skal være: "Om du legger inn tavlens adresse, vises gangavstanden fra tavlen
til hvert stoppested."

@SelmaBergstrand
Copy link
Collaborator

compress på mobil:

Screenshot 2025-02-27 at 14 13 01

@SelmaBergstrand
Copy link
Collaborator

Bug: å fjerne adresse-feltet fjerner ikke lenger gåavstand på tiles

Rettelse: oppdatering av gangavstand generelt har sluttet å virke. Det skjer ingenting når jeg skriver inn en ny adresse.

@SelmaBergstrand
Copy link
Collaborator

Avbryt-knappen gjør ingenting. Bør ikke den sette innstillingene tilbake til slik de var? Eller evt først trigge popup som sier at du har ulagrede endringer?

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