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

Ny teknisk plattform: MDX, Algolia-søk, og innlogging #158

Merged
merged 67 commits into from
Oct 18, 2022
Merged

Ny teknisk plattform: MDX, Algolia-søk, og innlogging #158

merged 67 commits into from
Oct 18, 2022

Conversation

mikaelbr
Copy link
Member

@mikaelbr mikaelbr commented Jan 5, 2022

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

  • Endre fra ren MD til MDX for komponenter som brukes.
  • Legge med ulike komponenter som kan brukes for kontorer osv.
  • Design til tab-menu for departments.
  • Legge til dokumentasjon for bruk av komponenter.

Søkemotor

  • Bruke søkemotor (ala Algolia) for bedre søk og mer statistikk på hva som søkes etter.
  • Samle statistikk på søk
  • Søk på avsnitt for å lenke til nærmeste overskrift eller direkte på avsnittet.
  • Skrive om algoritme til å fjerne duplikater for nøstede komponenter.
  • Støtte for avdelingsfilter på filnivå
  • Muliggjør synonymer
  • Design for søk

Login

  • Legge til optional innlogging for å kunne forhåndsdefinere kontor.
  • Design for visning av innlogging og utlogging.

@vercel
Copy link

vercel bot commented Jan 5, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/variant/handbook/7S2jwhTnSZCJuqvK3krwBjnFR1Up
✅ Preview: https://handbook-git-v3-variant1.vercel.app

@github-actions github-actions bot added the code label Jan 5, 2022
@mikaelbr mikaelbr changed the title WIP V3 Ny teknisk plattform: MDX, Algolia-søk, og innlogging Feb 23, 2022
@mikaelbr mikaelbr self-assigned this Feb 23, 2022
@adriansberg
Copy link
Member

Funker nå!

@mikaelbr
Copy link
Member Author

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.

@tormodhau
Copy link

Når vi er inne på mobiltesting:
B698D0DA-225D-44AE-9DD6-BF66265D37E8

@mikaelbr
Copy link
Member Author

Fixed

@tormodhau
Copy link

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?

@tormodhau tormodhau closed this Oct 17, 2022
@tormodhau tormodhau reopened this Oct 17, 2022
@tormodhau
Copy link

Det er godt mulig at dette er første gang jeg gjør pull requests i Github 😅😅

@adriansberg
Copy link
Member

@tormodhau Jeg har kun testet innlogging, har ikke gjort en review. 🫣

@tormodhau
Copy link

@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. 😄

@adriansberg
Copy link
Member

@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);

Choose a reason for hiding this comment

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

Jeg har lagt merke til at and brukes mye i kodebasene våre. Er det for å unngå den ekstra avhengigheten classnames innfører?

Copy link
Member Author

Choose a reason for hiding this comment

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

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 😄

isVisible,
});
}
return null;

Choose a reason for hiding this comment

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

^ 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"?

search-index/index.mjs Show resolved Hide resolved
currentSearch?: string;
}
// @TODO This should be automatically generated from the tree structure
const metadata = {
Copy link

@tormodhau tormodhau Oct 17, 2022

Choose a reason for hiding this comment

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

Fortjener denne en GIthub-issue, eller skal det ikke prioriteres med det første?

next.config.mjs Show resolved Hide resolved
<p>`[email protected]`</p>
</DepartmentItem>

<DepartmentItem dep="all">

Choose a reason for hiding this comment

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

Antar det må være en React.Node som children her, altså at det ikke kan være MDX?

Copy link
Member Author

Choose a reason for hiding this comment

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

Det kan være markdown også.

Copy link
Member Author

Choose a reason for hiding this comment

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

(mener jeg, men kan ta feil)

@adriansberg
Copy link
Member

image

Lite luft over filtrering når det kun er en avdeling å filtrere på.

@adriansberg
Copy link
Member

image

Riktig avdeling blir ikke satt som aktiv hvis man hopper fra søkeresultat til en seksjon. Må kanskje utvide isActiveHandbook.

Copy link
Member

@adriansberg adriansberg left a comment

Choose a reason for hiding this comment

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

Sett over det meste, ser greit ut 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

6 participants