Repository navigation
Hamed R. - #3
Hamed R.#3HamedRazizadeh-hub wants to merge 1 commit into
Conversation
| export interface Task { | ||
| id: string; | ||
| title: string; | ||
| description: string; | ||
| completed: boolean; | ||
| } | ||
|
|
||
| export type FilterStatus = "all" | "pending" | "completed"; | ||
|
|
||
| export interface TaskHandlers { | ||
| onAdd(title: string, description: string): void; | ||
|
|
||
| onToggle(id: string): void; | ||
|
|
||
| onDelete(id: string): void; | ||
|
|
||
| onEdit(id: string, title: string, description: string): void; | ||
|
|
||
| onFilter(status: FilterStatus): void; | ||
| } |
There was a problem hiding this comment.
This is very good and an professional approach on creating types files to isolate your definitions! 👏
| completed: boolean; | ||
| } | ||
|
|
||
| export type FilterStatus = "all" | "pending" | "completed"; |
There was a problem hiding this comment.
Adding a union type makes things more clear and not so generic as string. Good one!
| const form = root.querySelector<HTMLFormElement>(".todo-form")!; | ||
| const titleInput = root.querySelector<HTMLInputElement>("#title-input")!; | ||
| const descInput = root.querySelector<HTMLInputElement>("#desc-input")!; | ||
| const taskListEl = root.querySelector<HTMLUListElement>(".task-list")!; | ||
| this.filterBar = root.querySelector<HTMLDivElement>(".filter-bar")!; |
There was a problem hiding this comment.
Good use on the DOM elements here!
| if (!root) { | ||
| throw new Error("Root element not found"); | ||
| } |
There was a problem hiding this comment.
This is a really good one! This secure that the application will need to have the proper #app element
| tasks: Task[] = []; | ||
| store: TaskStore; |
There was a problem hiding this comment.
Since you are using class here, I would recommend to use private here to not expose this property outside. Read more here in this documentation and about encapsulation
| root: HTMLElement; | ||
| handlers: TaskHandlers; |
There was a problem hiding this comment.
Same thing as commented before about encapsulation
| const task = tasks.find((t) => t.id === id); | ||
| for (const li of this.root.querySelectorAll<HTMLElement>(".task-item")) { | ||
| const id = li.dataset.id!; | ||
| const task = tasks.find((t) => t.id === id)!; |
There was a problem hiding this comment.
Would be better to have a early return here if task is undefined?
rafasilpereira
added
Reviewed
|
The build generates asset paths as /assets/index.js (absolute), which breaks when dist/index.html is opened and that doesn't root at the project folder. Adding a vite.config.ts with base: './' fixes this so the built app works anywhere: |
2 participants
No description provided.