Port type widgets to React #7044
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a massive and very well-executed refactoring of the widget system to React. The new architecture is much cleaner, more modern, and more maintainable. The use of custom hooks to abstract complex logic is particularly well done. I've found a few issues, including a critical bug in one of the new utility functions, and some leftover debug statements. Once these are addressed, this will be an excellent contribution.
| }, [ note ]); | ||
|
|
||
| useTriliumEvent("executeWithContentElement", async ({ resolve, ntxId: eventNtxId}) => { | ||
| console.log("Got request for content ", ntxId, eventNtxId); |
There was a problem hiding this comment.
This console.log statement appears to be for debugging and should be removed.
There was a problem hiding this comment.
Pull Request Overview
This pull request represents a major refactoring effort to migrate type widgets from a legacy class-based architecture to a modern React/Preact component-based architecture. The changes include:
- Migration of type widgets (text, code, canvas, relation map, etc.) from class-based TypeWidget implementations to functional React components
- Reorganization of code into more modular structures with separation of concerns
- Introduction of new TypeScript interfaces and improved type safety
- Updates to dependencies and removal of transitive peer dependencies
- Code cleanup including removal of console.log statements and trailing whitespace
Reviewed Changes
Copilot reviewed 148 out of 156 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates to package dependencies, moving transitive peer dependencies |
| packages/commons/src/lib/server_api.ts | Adds new API response interfaces for attachment conversion, relation maps, and note maps |
| packages/commons/src/lib/attribute_names.ts | Reorganizes label types, moving note-type specific labels to a separate section |
| packages/ckeditor5/src/index.ts | Exports additional WatchdogState type |
| apps/server/src/routes/assets.ts | Adds Excalidraw asset serving for development mode |
| apps/server/src/routes/api/relation-map.ts | Refactors to use shared type definitions from commons package |
| apps/server/src/routes/api/attachments.ts | Adds type annotation using satisfies operator |
| apps/client/src/widgets/type_widgets/text/* | New React-based text editor implementation with mobile toolbar support |
| apps/client/src/widgets/type_widgets/code/* | New React-based code editor components |
| apps/client/src/widgets/type_widgets/canvas/* | Refactored canvas widget into React components with separated persistence logic |
| apps/client/src/widgets/type_widgets/relation_map/* | Complete rewrite of relation map as React components |
| apps/client/src/widgets/type_widgets/helpers/* | New split editor components for SVG and general split views |
| apps/client/src/widgets/type_widgets/options/* | Code cleanup and import path corrections |
| apps/client/src/widgets/type_widgets/type_widget.ts | Simplified to interface definition for type widget props |
| Various deleted .ts files | Legacy class-based type widget implementations removed |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/client/src/widgets/react/NoteLink.tsx:62
- Avoid automated semicolon insertion (92% of all statements in the enclosing function have an explicit semicolon).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/client/src/widgets/type_widgets/relation_map/RelationMap.tsx
Outdated
Show resolved
Hide resolved
apps/client/src/widgets/type_widgets/text/CKEditorWithWatchdog.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| <EditableTextTouchBar watchdogRef={watchdogRef} refreshTouchBarRef={refreshTouchBarRef} /> | ||
| </> | ||
| ) |
There was a problem hiding this comment.
Avoid automated semicolon insertion (95% of all statements in the enclosing function have an explicit semicolon).
eliandoran
force-pushed
the
react/type_widgets
branch
from
f811828 to
5f4d032
Compare
dosubot
bot
added
the
size:XL
Closes #2382.