Ny teknisk plattform: MDX, Algolia-søk, og innlogging #158
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/variant/handbook/7S2jwhTnSZCJuqvK3krwBjnFR1Up |
mikaelbr
changed the title
|
Funker nå! |
|
Det var nok for du manglet department i Azure AD. Ser nesten alle gjør det 😓 Mulig vi enten må bruke bemanning eller oppdatere Azure AD med rett data. |
|
Når vi er inne på mobiltesting: |
|
Fixed |
|
Jeg får også til å logge inn nå, men får «(none)» etter navnet. Antar jeg ikke har department i Azure. Samtidig kjenner jeg litt på at dette reviewet må lukkes snart - det er mange endringer! Hva tenker dere, @mikaelbr @adriansberg? Skal vi merge og heller rette småfeil som oppdages etterhvert? Om ingen får brukt innlogginga (og vi ikke tenker å gjøre noe med det rett etter merge), så kan kanskje knappen skjules midlertidig? |
|
Det er godt mulig at dette er første gang jeg gjør pull requests i Github 😅😅 |
|
@tormodhau Jeg har kun testet innlogging, har ikke gjort en review. 🫣 |
|
@adriansberg, jeg så gjennom koden tidligere i dag. Det er veldig mye, så noe særlig grundig gjennomgang har jeg ikke gjort. Prøver å finne en knapp for å "Approve", men det er visstnok ikke sånn Github PR funker... eller så er jeg blind. 😄 |
|
@tormodhau Skal være en grønn knapp øverst på sida, der det står "Add your review". 😄 Jeg kan se over koden nå. |
| @@ -101,8 +102,10 @@ export default function GeneralLayout({ | |||
| isActiveHandbook(category.path, asPath, true), | |||
| ); | |||
|
|
|||
| const classes = and(style.main, !noSidebar ? style.main__sidebar : undefined); | |||
|
|
|||
There was a problem hiding this comment.
Jeg har lagt merke til at and brukes mye i kodebasene våre. Er det for å unngå den ekstra avhengigheten classnames innfører?
There was a problem hiding this comment.
Glad i økosystem jeg altså, men tenker at for utils som er på en linje så trenger vi ikke flere innslag i lockfiles og package files enn vi har i kildekodelinjer 😄
src/components/department/index.tsx
Outdated
| isVisible, | ||
| }); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
^ linje 58-71
Her synes jeg nøstingen går litt ut over lesbarheten. Foreslår å gjøre begge if-ene på toppen til Guard Clauses, så blir strukturen helt "flat"?
| currentSearch?: string; | ||
| } | ||
| // @TODO This should be automatically generated from the tree structure | ||
| const metadata = { |
There was a problem hiding this comment.
Fortjener denne en GIthub-issue, eller skal det ikke prioriteres med det første?
| <p>`faktura.trondheim@variant.no`</p> | ||
| </DepartmentItem> | ||
|
|
||
| <DepartmentItem dep="all"> |
There was a problem hiding this comment.
Antar det må være en React.Node som children her, altså at det ikke kan være MDX?
There was a problem hiding this comment.
Det kan være markdown også.
There was a problem hiding this comment.
(mener jeg, men kan ta feil)
|
Lite luft over filtrering når det kun er en avdeling å filtrere på. |
|
Riktig avdeling blir ikke satt som aktiv hvis man hopper fra søkeresultat til en seksjon. Må kanskje utvide |
Dette er kun en teknisk endring for underliggende teknologi for håndboken. Det er ikke store endringer i design, innhold eller struktur. Men det er et steg på vegen i arbeid som foregår. Målet her er først og fremst å ha et søk som gjør at vi kan se hva som blir søkt på slik at vi kan bruke det til innsikt i strukturering. Samtidig foregår det arbeid på omstrukturering for å lettere løfte frem innhold som er relevant. Det er også noe designløft som kommer til å gjøres etterhvert.
Dette skal med andre ord fungere ca som før, men forhåpentligvis litt bedre. Så utnytter vi ikke per nå det maksimale potensiale, men jeg ønsker å merge dette nå slik at det ikke blir enda mer langtlevende branch.
For å muliggjøre neste generasjon av Håndbok i et iterativt løp så er det en del tekniske endringer som gjøres:
MDX, rike komponenter
Søkemotor
Login