From 945dcdead078b12e657959dc6f6ca18d92faa0a9 Mon Sep 17 00:00:00 2001 From: Yura Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Fri, 15 May 2026 13:37:19 +0200 Subject: [PATCH] Introduce README.md, AI-USE.md. Setup nix-shell for node. Tag personal comments with `@PERSONAL-NOTE`. --- .envrc | 1 + .gitignore | 2 ++ AI-USE.md | 21 +++++++++++++++++++ README.md | 47 ++++++++++++++++++++++++++++++++++++++++++ shell.nix | 7 +++++++ src/ui/Picturarium.tsx | 23 ++++++++++++++++----- 6 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 .envrc create mode 100644 AI-USE.md create mode 100644 README.md create mode 100644 shell.nix diff --git a/.envrc b/.envrc new file mode 100644 index 0000000..1d953f4 --- /dev/null +++ b/.envrc @@ -0,0 +1 @@ +use nix diff --git a/.gitignore b/.gitignore index 9a5ee26..e06f623 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ yarn-error.log* pnpm-debug.log* .DS_Store + +.direnv diff --git a/AI-USE.md b/AI-USE.md new file mode 100644 index 0000000..466c05a --- /dev/null +++ b/AI-USE.md @@ -0,0 +1,21 @@ +Wrote most of the code by hand. + +My own design: + +- config/state/msg/update/effects, all the types +- which hooks to use +- remote/request/result +- splitting the codebase into modules/folders/files + +LLM-generated: +where I mostly reviewed + +- The Material UI / styling - after using a few components manually, I mostly let LLM decide which components to use, how to use them, and letting it decide/generate styling details. + +LLM-assisted: + +- Name suggestions +- Project setup (linters, formatters, ts compiler settings) +- Checking and Code Review +- Snippet generation +- Explanations of external API details, and confirming browser caching behaviour diff --git a/README.md b/README.md new file mode 100644 index 0000000..366cb30 --- /dev/null +++ b/README.md @@ -0,0 +1,47 @@ +# Picturarium + +Picturarium is a small React image gallery using the Lorem Picsum API. It displays paginated image thumbnails, caches loaded pages during the session, and opens selected images in a larger +modal view. The app uses TypeScript, Material UI, and explicit remote/request state handling for loading and error states. + +# Installation + +This assumes `node` is installed on your system. +TODO: specify `git clone` too here. + +```bash +git clone // TODO +cd picturarium +npm install +``` + +# Execution + +After cloning the repo in `picturarium/`: + +## dev + +You can start a vite dev-server + +```bash +npm run dev +``` + +## production + +To see production build: + +```bash +npm run build +npm run preview +``` + +# Comments + +In this codebase I sometimes use comments that start as + +```typescript +// @PERSONAL_NOTE +``` + +These are just there for this particular assignment, and I wouldn't included them in a real codebase. +They are just notes to let you know why I made specific decisions and how I think about stuff. diff --git a/shell.nix b/shell.nix new file mode 100644 index 0000000..f2a16d2 --- /dev/null +++ b/shell.nix @@ -0,0 +1,7 @@ +{ pkgs ? import {} }: + +pkgs.mkShell { + packages = [ + pkgs.nodejs_22 + ]; +} diff --git a/src/ui/Picturarium.tsx b/src/ui/Picturarium.tsx index 4ca89e9..e9c812f 100644 --- a/src/ui/Picturarium.tsx +++ b/src/ui/Picturarium.tsx @@ -5,6 +5,7 @@ import { Result } from "../result" import { RequestError } from "../request" import { Remote } from "../remote" +// @PERSONAL_NOTE // NOTE: There are multiple school of thoughts on how to structure a component. // - What I prefer is to extract generic components or complex sub-components into their own modules (but this assignment is too simple for that). // I also like to keep views/states/messages/initialization/effects/update of one component in one single (even if giant) file (or a structure like `Component/...` + `Component.tsx`) @@ -41,6 +42,7 @@ const CONFIG = { // desktopRows: 3, // } as const satisfies Config +// @PERSONAL_NOTE // Could do comptime assert: `size = mobile product`, and `size = desktop product`, but this is pretty heavy in TS. assertValidConfig(CONFIG) @@ -81,6 +83,10 @@ const Dimension = { // === Page === type PageIndex = number +// @PERSONAL_NOTE +// This is a huge overkill for this type of application - especially because page size is known at compile-time and we can't even change page size at runtime. +// Most of the identity issues down-the line would be trivial if we just used `PageIndex` instead (which is a nice immutable value where equality is structural). +// But in general there's no avoiding objects in js/ts, so during code-review this would allow me to demonstrate understanding of these subtle issues. That's why I chose to use it here. type Page = { index: PageIndex size: number @@ -97,6 +103,7 @@ const Page = { return { ...page, index: page.index + 1 } }, previous(page: Page): Page { + // @PERSONAL_NOTE // Could do // if (page.index == FIRST_PAGE) { return page } // this preserves identity of object, so is nicer for `useEffect`, but it's waaaay to subtle. So I'm not relying on that. @@ -158,6 +165,7 @@ function useApp(): [State, Dispatch] { const [state, dispatch] = useReducer(update, State.init()) // === Caching API calls === + // @PERSONAL_NOTE // Could also cache `Promise` in case two requests are made really fast one after another (not really the case in this app so whatever) const cacheRef = useRef>(new Map()) @@ -178,11 +186,16 @@ function useApp(): [State, Dispatch] { } // === initialization & reloading === - useEffect(() => { - void getImageIdsCached(state.page).then((result) => { - dispatch({ tag: "imagesReceived", forPage: state.page, result }) - }) - }, [state.page, state.refreshSignal]) // Would have been amazing if we could put `Page.key(state.page)` inside of this. Then the trouble with identity would be gone. But we can't, because React compiler and linter would complain /facepalm + useEffect( + () => { + void getImageIdsCached(state.page).then((result) => { + dispatch({ tag: "imagesReceived", forPage: state.page, result }) + }) + }, + // @PERSONAL_NOTE + // Would have been amazing if we could put `Page.key(state.page)` inside of this. Then the trouble with identity would be gone. But we can't, because React compiler and linter would complain /facepalm + [state.page, state.refreshSignal], + ) function update(state: State, msg: Msg): State { switch (msg.tag) {