-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Web version pt.3 #1722
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
Web version pt.3 #1722
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 42b487f in 54 seconds
More details
- Looked at
29
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/index.tsx:27
- Draft comment:
Uncommented handleCanvasMouseDown. Ensure this behavior is intended for cases when event.target doesn't match containerRef.current. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/web/src/app/project/[id]/_components/canvas/index.tsx:27
- Draft comment:
Consider using event.currentTarget instead of containerRef.current to ensure correct event target reference. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/index.tsx:27
- Draft comment:
Uncommented handleCanvasMouseDown is now active for clearing the UI when clicking the canvas background. Ensure that triggering clearUI only when event.target equals containerRef.current is the intended behavior. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. apps/web/src/app/project/[id]/_components/canvas/index.tsx:1
- Draft comment:
The PR description mentions onboarding docs improvements (Next.js, Remix, Webpack, Babel), but this diff only reactivates a UI handler. Ensure that documentation changes are included in the overall update. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/web/src/app/project/[id]/_components/canvas/index.tsx:49
- Draft comment:
It appears that 'lintedScale' might be a typo and was intended to be 'clampedScale', especially since it’s the result of the clampZoom function. Please review and consider renaming it for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_oTUM4uXxallIE1wm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on dbd547c in 43 seconds
More details
- Looked at
103
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
11
drafted comments based on config settings.
1. apps/studio/src/routes/editor/Canvas/index.tsx:151
- Draft comment:
Consider including middleMouseButtonDown and middleMouseButtonUp in the useEffect dependency array since they are added as event listeners. This ensures updated callbacks are used if dependencies change. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. apps/web/src/app/project/[id]/_components/canvas/index.tsx:22
- Draft comment:
The fixed scale and position (set to 1 and {0, 0}) along with commented out engine updates in handleZoom suggest this feature is not fully implemented. Verify if this is intentional or a temporary stub. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. apps/web/src/app/project/[id]/_components/canvas/index.tsx:116
- Draft comment:
For consistency with the studio version, consider wrapping middleMouseButtonDown/up in useCallback to optimize performance and consistency. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. apps/web/src/app/project/[id]/_components/canvas/index.tsx:1
- Draft comment:
The PR title and description mention adding onboarding docs, but this diff only includes canvas interaction changes. Please ensure the PR covers all intended features. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/studio/src/routes/editor/Canvas/index.tsx:151
- Draft comment:
Include middleMouseButtonDown/up in the useEffect dependency array so the event listeners always use the latest handler references. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. apps/web/src/app/project/[id]/_components/canvas/index.tsx:24
- Draft comment:
Using hardcoded scale and position (scale = 1, position = {x: 0, y: 0}) prevents dynamic updates from zoom/pan logic. Consider using editorEngine.canvas values. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. apps/web/src/app/project/[id]/_components/canvas/index.tsx:51
- Draft comment:
Updates to canvas scale and position in handleZoom are commented out. If zoom functionality is desired, these assignments should be re-enabled. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. apps/web/src/app/project/[id]/_components/canvas/index.tsx:116
- Draft comment:
Wrap middleMouseButtonDown (and similarly middleMouseButtonUp) in useCallback to ensure consistent function references for addEventListener and cleanup. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. apps/web/src/app/project/[id]/_components/canvas/index.tsx:117
- Draft comment:
Ensure consistency in how editor mode is updated; here 'editorEngine.state.editorMode' is used while the studio version uses 'editorEngine.mode'. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
10. apps/studio/src/routes/editor/Canvas/index.tsx:48
- Draft comment:
Typo alert: The variable name 'lintedScale' might be a mistake. It appears you intended to name it 'clampedScale' to better reflect that the scale value is clamped within allowed limits. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. apps/web/src/app/project/[id]/_components/canvas/index.tsx:45
- Draft comment:
Typo notice: The variable name 'lintedScale' (line 46) appears to be a typo. It is likely meant to be 'clampedScale' to be consistent with its usage in clamping the scale value. Consider renaming it for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_E33GWg1Zxc144xOG
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on f720951 in 2 minutes and 46 seconds
More details
- Looked at
145
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
11
drafted comments based on config settings.
1. apps/studio/src/routes/editor/Canvas/index.tsx:26
- Draft comment:
Consider memoizing handleCanvasMouseDown with useCallback if it's passed down as a prop to avoid unnecessary re-renders. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/studio/src/routes/editor/Canvas/index.tsx:115
- Draft comment:
Middle mouse button handlers are now inline. Consider using useCallback for consistency and to prevent re-creation on each render. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/index.tsx:22
- Draft comment:
The scale and position values are hardcoded (scale = 1, position = {x: 0, y: 0}). Confirm that this is intended for the web version. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/src/app/project/[id]/_components/canvas/index.tsx:163
- Draft comment:
Overlay and PanOverlay components are commented out. Make sure this is intended before merging. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/web/src/app/project/[id]/_components/canvas/index.tsx:116
- Draft comment:
Inconsistent API: using editorEngine.state.editorMode in web vs editorEngine.mode in studio. Consider aligning these APIs for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The core API usage (editorEngine.state.editorMode vs editorEngine.mode) wasn't changed in this PR - only the useCallback wrapper was removed. The inconsistency may be real, but it's a pre-existing issue. Comments should focus on changes made in the PR, not pre-existing issues. Cross-file issues should also be ignored per the rules.
The API inconsistency could be a real issue that impacts maintainability. Maybe the useCallback removal makes this a good time to fix it?
While the inconsistency may be real, this PR is just removing useCallback. Raising cross-file API issues would be better done in a dedicated PR focused on API consistency.
Delete the comment because it addresses a pre-existing cross-file issue rather than changes made in this PR.
6. apps/studio/src/routes/editor/Canvas/index.tsx:115
- Draft comment:
Consider memoizing the middleMouseButtonDown and middleMouseButtonUp functions (e.g. using useCallback) so that the event listener references remain stable for addEventListener/removeEventListener in useEffect. This prevents potential stale closure issues when dependencies change. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The functions do use state/context values that could change (editorEngine and setIsPanning). The functions are used in event listeners that are added/removed in useEffect. However, the useEffect dependency array only includes handleWheel, suggesting these mouse handlers are intentionally meant to always reference the latest values. The change from useCallback to regular functions was likely intentional.
I could be wrong about the dependency array being intentionally limited - it might be a bug. Also, re-creating these functions on every render could cause unnecessary event listener churn.
The event listener churn would have minimal performance impact, and React's batching would ensure the latest state values are always used. The limited dependency array appears intentional given the pattern used throughout the file.
The comment raises a theoretical concern but there's no strong evidence of an actual problem. The current implementation appears intentional and functional.
7. apps/studio/src/routes/editor/Canvas/index.tsx:1
- Draft comment:
The PR title and description mention onboarding documentation improvements, but this diff only shows canvas interaction updates. Please ensure that the intended documentation changes are included or referenced. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. apps/web/src/app/project/[id]/_components/canvas/index.tsx:116
- Draft comment:
Similarly, consider memoizing middleMouseButtonDown and middleMouseButtonUp with useCallback to ensure they remain stable when attached via addEventListener. This aligns with best practices for consistent cleanup in useEffect. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The functions are attached via addEventListener and removed via removeEventListener. While useCallback could prevent unnecessary re-renders, the cleanup is already handled properly in the useEffect. The functions are simple and the performance impact would be minimal. The useEffect dependency array only includes handleWheel, suggesting these handlers don't need to be dependencies.
The comment has some merit since it follows React best practices for event handlers. However, in this case the optimization may be premature and add unnecessary complexity.
Given the simple nature of these handlers and proper cleanup already in place, the benefit of adding useCallback here is minimal and would add complexity without clear performance gains.
While technically correct, this suggestion is not important enough to warrant a comment. The current implementation works fine and the optimization would have minimal impact.
9. apps/web/src/app/project/[id]/_components/canvas/index.tsx:45
- Draft comment:
In handleZoom, the assignments to update editorEngine.canvas.scale and canvas.position are commented out while scale and position are hard-coded as constants. This makes the zoom/pan computations ineffective. If these are placeholders, consider removing them or adding a clarifying comment. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. apps/studio/src/routes/editor/Canvas/index.tsx:45
- Draft comment:
Typographical issue: The variable 'lintedScale' (line 45) seems to be misnamed. It appears that 'clampedScale' would be a more descriptive name since it is the result of the clampZoom function. Consider renaming for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. apps/web/src/app/project/[id]/_components/canvas/index.tsx:46
- Draft comment:
Typographical error: It appears that 'lintedScale' is a typo. Consider renaming it to 'clampedScale' to maintain consistency and clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_BaIjLRH495R4tZFs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on b11d6bf in 40 seconds
More details
- Looked at
254
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/top-bar/index.tsx:44
- Draft comment:
Ensure TooltipProvider is consistently used. If tooltips are needed app-wide, consider centralizing TooltipProvider to avoid wrapping individual components. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/web/src/app/project/[id]/_components/top-bar/mode-toggle.tsx:16
- Draft comment:
The use of getMode for state initialization is clear now. Consider renaming it to improve clarity if additional logic is added in the future. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/top-bar/index.tsx:17
- Draft comment:
Remove commented-out code (e.g. editorEngine lines) if it's no longer needed. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. apps/web/src/app/project/[id]/_components/top-bar/index.tsx:20
- Draft comment:
Consider moving the static UNDO_REDO_BUTTONS array outside the component to prevent re-creation on every render. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. apps/web/src/app/project/[id]/_components/top-bar/mode-toggle.tsx:13
- Draft comment:
Cleanup commented-out code (editorEngine integration and TODO notes) once integration is finalized for clarity. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. apps/web/src/app/project/[id]/_components/top-bar/mode-toggle.tsx:36
- Draft comment:
Ensure the getMode function can handle additional editor modes if they are introduced in the future. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. apps/web/src/app/project/[id]/_components/top-bar/index.tsx:44
- Draft comment:
Verify that wrapping the component with TooltipProvider does not create redundant or nested providers when integrated with other components. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_xdnQkbUrl59YiubS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on c80ca9d in 1 minute and 39 seconds
More details
- Looked at
389
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
12
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/index.tsx:28
- Draft comment:
Remove temporary override for scale and position; it overrides editorEngine values. - Reason this comment was not posted:
Comment was on unchanged code.
2. apps/web/src/app/project/[id]/_components/canvas/index.tsx:69
- Draft comment:
Ensure updating of canvas.position isn’t omitted; commented assignment seems intentional? - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. apps/web/src/app/project/[id]/_components/canvas/index.tsx:70
- Draft comment:
Review dependencies in useCallback; using editorEngine.canvas may not trigger updates if its reference changes. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/src/app/project/[id]/_components/canvas/overlay/pan.tsx:43
- Draft comment:
Good use of MobX observer; ensure pan logic respects current canvas position. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. apps/web/src/app/project/[id]/_components/top-bar/index.tsx:16
- Draft comment:
Remove unused commented imports to keep the file clean. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. apps/web/src/app/project/[id]/_components/top-bar/mode-toggle.tsx:30
- Draft comment:
Consider renaming 'getNormalizedMode' to clarify its role in mapping editorMode to available modes. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. apps/web/src/components/store/editor/engine/index.ts:32
- Draft comment:
Remove commented-out code if it’s no longer needed to improve maintainability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The commented code appears to be part of an active refactoring effort, not abandoned code. The systematic nature of the comments and the fact that some parts are still active suggests this is a work in progress. Removing comments now could interfere with the refactoring process. The commented code likely serves as a reference for the ongoing changes.
The comment could be right if this was dead code, but what if this is an intentional intermediate state during refactoring? The systematic nature of the comments suggests purpose rather than neglect.
Given the systematic commenting out and partial active code, this appears to be an intentional intermediate state during refactoring rather than dead code that should be removed.
The comment should be deleted as it's likely interfering with an active refactoring process where the commented code is still serving a purpose.
8. apps/web/src/app/project/[id]/_components/canvas/index.tsx:56
- Draft comment:
editorEngine.canvas is referenced here, but its implementation appears to be commented out in EditorEngine. Ensure that the canvas property is available or update these references. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. apps/web/src/app/project/[id]/_components/canvas/overlay/pan.tsx:27
- Draft comment:
Verify browser compatibility for event.movementX/movementY; consider providing a fallback if needed. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
10. apps/web/src/components/store/editor/engine/index.ts:33
- Draft comment:
The canvas manager is commented out in EditorEngine, yet canvas properties are used in canvas components. Re-enable CanvasManager or update the references to maintain consistent state management. - Reason this comment was not posted:
Comment was on unchanged code.
11. apps/web/src/app/project/[id]/_components/canvas/index.tsx:50
- Draft comment:
It looks like 'lintedScale' might be a typo. Perhaps it was meant to be 'clampedScale', since it's the result of the clampZoom function. Changing it could improve clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. apps/web/src/app/project/[id]/_components/canvas/overlay/pan.tsx:43
- Draft comment:
The string literal 'visible ' on line 43 appears to have a trailing space. If this space is not intentional for styling purposes, consider removing it to maintain consistency and avoid potential issues with class name matching. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While trailing spaces in class names could potentially cause issues, this seems like a very minor issue. The cn() utility from @onlook/ui-v4/utils likely handles class name normalization. Even if it doesn't, a trailing space in a class name is unlikely to cause any real problems in modern browsers and CSS frameworks. This feels more like a nitpick than a substantial issue requiring a code change.
The trailing space could theoretically cause class name comparison issues or unnecessary whitespace in the DOM. Some CSS frameworks or testing tools might be sensitive to exact class name matching.
Modern frontend tooling and the cn() utility would likely normalize whitespace. The impact is negligible and doesn't warrant a code change or review comment.
This comment should be deleted as it points out an extremely minor issue that's unlikely to cause problems and doesn't require action.
Workflow ID: wflow_GlmLgVWT1nIivbxl
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
||
const editorEngine = useEditorEngine(); | ||
const containerRef = useRef<HTMLDivElement>(null); | ||
const [isPanning, setIsPanning] = useState(false); | ||
// const scale = editorEngine.canvas.scale; | ||
// const position = editorEngine.canvas.position; | ||
|
||
// TODO: Remove this | ||
const scale = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove dummy scale/position; use real canvas state from editorEngine
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on f26ed95 in 1 minute and 50 seconds
More details
- Looked at
404
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/hotkeys/index.tsx:15
- Draft comment:
Good implementation of hotkeys for zoom reset. Consider removing commented-out code if not needed. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. apps/web/src/app/project/[id]/_components/canvas/index.tsx:28
- Draft comment:
Temporary constants for 'scale' and 'position' are used. Remove them when integrating actual state from editorEngine. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/index.tsx:29
- Draft comment:
Hardcoded 'scale' and 'position' values are used (const scale = 1; position = { x: 0, y: 0 }). This prevents dynamic updates from editorEngine.canvas. Remove these hardcoded values and use the engine state instead. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/src/components/store/editor/engine/window/index.ts:10
- Draft comment:
The condition in 'isWindowSelected' is contradictory. It checks that the selected elements array has a length greater than 0 and is simultaneously equal to 0, which will always be false. - Reason this comment was not posted:
Marked as duplicate.
5. apps/web/src/app/project/[id]/page.tsx:15
- Draft comment:
The PR description mentions onboarding docs improvements, but no documentation changes are observed in this diff. Please ensure that the intended documentation updates are provided. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to ensure that the intended documentation updates are provided, which is similar to asking them to confirm their intention or to ensure something is done. This violates the rule against asking the author to confirm their intention or ensure something is done. Therefore, this comment should be removed.
Workflow ID: wflow_bc5I7ESxl19YXCbR
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@@ -7,7 +7,7 @@ export class WindowManager { | |||
} | |||
|
|||
get isWindowSelected() { | |||
// return this.webviews.selected.length > 0 && this.elements.selected.length === 0; | |||
return this.editorEngine.elements.selected.length > 0 && this.editorEngine.elements.selected.length === 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logical bug: the condition checks if elements.selected.length > 0
and equals 0
simultaneously, which always returns false.
return this.editorEngine.elements.selected.length > 0 && this.editorEngine.elements.selected.length === 0; | |
return this.editorEngine.elements.selected.length > 0; |
// private imageManager: ImageManager; | ||
// private themeManager: ThemeManager; | ||
// private fontManager: FontManager; | ||
readonly canvas: CanvasManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several manager properties (e.g., canvas, chat, webview, code, pages, error, image, theme, font) are declared but never instantiated. This may lead to runtime errors when these properties are accessed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 730a333 in 1 minute and 22 seconds
More details
- Looked at
246
lines of code in11
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. apps/web/src/components/store/editor/engine/window/index.ts:11
- Draft comment:
Logical bug: The condition checks if elements.selected.length > 0 && elements.selected.length === 0, which is always false. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. apps/web/src/app/projects/_components/edit-app.tsx:1
- Draft comment:
PR description mentions onboarding docs improvements (issue 123), but no documentation changes are present in this diff. Please ensure onboarding docs updates are included if intended. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that documentation updates are included, which is similar to asking them to confirm their intention or ensure something is done. This violates the rule against asking the author to confirm intentions or ensure changes are made.
3. apps/web/src/components/store/editor/engine/chat/code.ts:14
- Draft comment:
The constructor still uses 'projectsManager: ProjectsManager' but the import was removed. Either remove the unused parameter or add the appropriate import. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/src/components/store/editor/engine/index.ts:34
- Draft comment:
EditorEngine declares a property 'webview' but elsewhere (e.g., in action manager) it's referenced as 'webviews'. This inconsistency may lead to runtime errors; consider renaming for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/web/src/components/store/editor/engine/window/index.ts:10
- Draft comment:
The getter 'isWindowSelected' uses a contradictory condition: it checks for length > 0 and length === 0 simultaneously, which is always false. Please fix the logic. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_WWaOn9olnJNKpTqK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 5690cce in 1 minute and 15 seconds
More details
- Looked at
89
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. apps/web/src/app/projects/_components/select.tsx:18
- Draft comment:
Remove TODO comment and update mock projects once integration is complete. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/web/src/app/projects/_components/select.tsx:43
- Draft comment:
Ensure new project addition aligns with sorting logic and onboarding flow. - Reason this comment was not posted:
Confidence changes required:10%
<= threshold50%
None
3. apps/web/src/components/store/editor/engine/index.ts:62
- Draft comment:
Verify removal of 'projectsManager' dependency in CanvasManager constructor. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. apps/web/src/app/projects/_components/select.tsx:18
- Draft comment:
Mock data is still being used (marked TODO). Ensure to replace with projectsManager logic before production. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/web/src/app/projects/_components/select.tsx:57
- Draft comment:
sortProjects mutates the input array. Consider cloning the array before sorting to avoid side-effects. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
6. apps/web/src/app/projects/page.tsx:8
- Draft comment:
Review the use of 'overflow-hidden' on the container to ensure it aligns with the intended design. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. apps/web/src/components/store/editor/engine/index.ts:62
- Draft comment:
CanvasManager is now instantiated without passing projectsManager. Confirm that its constructor has been updated to reflect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to confirm a change in the constructor of CanvasManager. This is a request for confirmation, which violates the rule against asking the author to confirm their intention or to ensure behavior is intended.
Workflow ID: wflow_RJHhGT0y8xYhum45
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 521c0d7 in 2 minutes and 5 seconds
More details
- Looked at
478
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
10
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/frames/index.tsx:7
- Draft comment:
Use a unique key (like settings.id) instead of the index for Frame components. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50%
Using array indices as React keys is generally considered an anti-pattern that can cause issues with component re-rendering and state management, especially when items can be reordered or deleted. However, I don't have visibility into the settings object structure to confirm that settings.id exists. The comment makes an assumption about the data structure.
I don't have evidence that settings.id actually exists in the data structure. The frames array might be static or the index might be a valid unique identifier in this specific case.
While using index as a key isn't ideal, without seeing the settings type definition or knowing the frame's data structure, I can't be certain that settings.id is available or that using index would cause actual problems here.
While the comment raises a valid React best practice, we lack strong evidence that the suggested solution is correct or that the current implementation causes real issues.
2. apps/web/src/app/project/[id]/page.tsx:12
- Draft comment:
The issue description mentions adding onboarding documentation, but this PR only adds a Frames UI. Confirm if the documentation changes are in a separate PR. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. apps/web/src/components/store/editor/engine/canvas/index.ts:20
- Draft comment:
Default frame settings in the CanvasManager constructor are hardcoded. Verify these defaults meet all project requirements. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/src/app/project/[id]/_components/canvas/frames/resize-handles.tsx:6
- Draft comment:
Multiple elements are using the same 'resizeHandleRef'. Remove or assign separate refs if you plan to use them. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The ref is never actually used in the code - it's defined but there are no operations that read or manipulate the ref. The resize functionality works through event listeners rather than ref manipulation. Since the ref isn't being used for anything, having the same ref on multiple elements doesn't cause any actual problems.
The comment correctly identifies that the same ref is used multiple times, which is generally not a good practice. Maybe the ref is used somewhere else in the codebase that we can't see?
While using the same ref multiple times is generally not good practice, in this specific case the ref isn't actually used for anything, so it's not causing any problems. The comment is technically correct but points out an issue that has no practical impact.
The comment should be deleted because while it points out a real pattern, that pattern isn't causing any actual problems in the code. The ref appears to be unused and could probably be removed entirely.
5. apps/web/src/app/project/[id]/_components/canvas/frames/resize-handles.tsx:33
- Draft comment:
Consider cleaning up the window event listeners (mousemove and mouseup) on unmount to prevent potential memory leaks. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
6. apps/web/src/app/project/[id]/_components/canvas/frames/resize-handles.tsx:20
- Draft comment:
The 'lockedPreset' state is initialized but never updated. Remove it if it's not used or implement its functionality. - Reason this comment was not posted:
Comment looked like it was already resolved.
7. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:5
- Draft comment:
The Frame component uses a 'fixed' position while being rendered inside a grid container. Confirm that this layout behavior is intentional. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
8. apps/web/src/app/project/[id]/page.tsx:2
- Draft comment:
The PR description mentions adding onboarding docs for frameworks, but no documentation changes are present. Please ensure the docs updates are included or update the PR description accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment asks the PR author to update the PR description or ensure documentation updates are included. It violates the rule against asking the author to update the PR description and indirectly asks for confirmation of intention regarding documentation changes.
9. apps/web/src/components/store/editor/engine/canvas/index.ts:84
- Draft comment:
Verify that 'Orientation.Potrait' is the intended value; typically, 'Portrait' is used. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
10. apps/web/src/app/project/[id]/_components/canvas/index.tsx:47
- Draft comment:
It appears that 'lintedScale' may be a typographical error. Consider renaming it to something more descriptive like 'clampedScale' to match its purpose (obtained from clampZoom) and to avoid confusion. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_43DqGipbpHvTq8Wh
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
})} | ||
> | ||
<div | ||
ref={resizeHandleRef} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple elements share the same ref (resizeHandleRef
). Remove or use unique refs if needed.
apps/web/src/app/project/[id]/_components/canvas/frames/index.tsx
Outdated
Show resolved
Hide resolved
dimension: { width: 1000, height: 1000 }, | ||
device: 'Desktop', | ||
theme: Theme.Light, | ||
orientation: Orientation.Potrait, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: Orientation.Potrait
should likely be corrected to Orientation.Portrait
.
orientation: Orientation.Potrait, | |
orientation: Orientation.Portrait, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on a41b162 in 52 seconds
More details
- Looked at
25
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:1
- Draft comment:
Removed unused useEditorEngine and hovered state; ensure no dependent code is missing. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:21
- Draft comment:
Consider cleaning up commented-out code if it's no longer needed. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:1
- Draft comment:
Good cleanup: removed unused hooks (useEditorEngine and useState) since they weren’t being used. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:21
- Draft comment:
Consider removing or refactoring commented-out code (e.g., BrowserControls and GestureScreen) to improve clarity. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. apps/web/src/app/project/[id]/_components/canvas/frames/frame.tsx:1
- Draft comment:
Note: Issue 123 mentions onboarding docs improvements. This file shows UI clean-up only – ensure documentation updates are included elsewhere in the PR. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_XCRkH43sUwk3PToc
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on e528802 in 2 minutes and 3 seconds
More details
- Looked at
130
lines of code in7
files - Skipped
0
files when reviewing. - Skipped posting
8
drafted comments based on config settings.
1. packages/ui-v4/src/components/icons/index.tsx:1448
- Draft comment:
The file contains a lot of repeated inline SVG attributes (e.g. fill, style objects). Consider extracting common constants or creating a wrapper to avoid duplication and ease maintenance. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
2. packages/ui/src/components/icons/index.tsx:1448
- Draft comment:
This extensive file also duplicates many inline SVG attributes. Consider refactoring shared properties into reusable components or constants to promote DRY principles and improve maintainability. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. packages/ui-v4/src/components/icons/index.tsx:135
- Draft comment:
Consider adding aria-hidden="true" (or an appropriate role) to SVG elements if they are purely decorative. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/ui-v4/src/components/icons/index.tsx:1369
- Draft comment:
Verify that the gradient stop color definitions using CSS variables (e.g. var(--color-gray-50)) are consistent with your design system. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
5. packages/ui/src/components/icons/index.tsx:130
- Draft comment:
Consider adding aria-hidden="true" to decorative SVG icons for better accessibility. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. packages/ui/src/components/icons/index.tsx:1369
- Draft comment:
Ensure consistency in gradient stop definitions: here using Tailwind's theme() (e.g. theme(colors.gray.50)) differs from the CSS variable approach in ui-v4. Confirm if this difference is intentional. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
7. apps/studio/src/routes/editor/LayersPanel/WindowsTab/FrameDimensions.tsx:380
- Draft comment:
Typographical error: In the Landscape button, the orientation is compared with the string "Landscape" instead of using the Orientation.Landscape constant. This could lead to unexpected behavior. Please update it to use Orientation.Landscape for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. packages/models/src/constants/editor.ts:48
- Draft comment:
Good fix spotting a typo: 'Potrait' has been corrected to 'Portrait' in the Orientation enum definition and its usage in the DefaultSettings object. This change improves clarity and consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only acknowledges a typo correction without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
Workflow ID: wflow_1LlCrXIEjXNMeUVp
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on a86859e in 2 minutes and 43 seconds
More details
- Looked at
2309
lines of code in16
files - Skipped
0
files when reviewing. - Skipped posting
10
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/index.tsx:47
- Draft comment:
Consider renaming 'lintedScale' to 'clampedScale' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/measurement.tsx:124
- Draft comment:
Consider reviewing performance of heavy calculations in the 'distances' useMemo; memoization is used, but if many measurements occur, performance may be impacted. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/rect/resize.tsx:159
- Draft comment:
Consider moving DOUBLE_CLICK_TIMEOUT constant outside the component to avoid re-creation on each render. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While technically correct that the constant is recreated on each render, this is a very minor optimization. The constant is a simple number value that takes negligible memory and CPU time to create. JavaScript engines are highly optimized for such cases. The current location also keeps the constant close to where it's used, which has readability benefits.
The comment is technically accurate - the constant is recreated on each render. Moving it could provide a tiny performance benefit.
However, the performance impact is so negligible that it doesn't justify a code change or review comment. The readability benefit of keeping the constant near its usage outweighs the microscopic performance cost.
This comment should be removed as it suggests an optimization that would provide negligible benefits while potentially reducing code readability.
4. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/text.tsx:80
- Draft comment:
Ensure the dependency array for the ProseMirror setup useEffect covers all needed dependencies to avoid stale state. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
5. apps/web/src/app/components/store/editor/engine/index.ts:107
- Draft comment:
Consider cleaning up commented code if no longer needed to improve readability. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
6. apps/web/src/components/store/index.ts:15
- Draft comment:
Ensure that the UserManager is correctly instantiated and integrated, since it now affects language and settings. - Reason this comment was not posted:
Confidence changes required:40%
<= threshold50%
None
7. apps/web/src/app/project/[id]/_components/canvas/index.tsx:47
- Draft comment:
Rename 'lintedScale' to 'clampedScale' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx:82
- Draft comment:
Ternary for 'animationClass' returns identical values regardless of condition. Adjust to use different classes if intended. - Reason this comment was not posted:
Marked as duplicate.
9. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/text.tsx:36
- Draft comment:
Reinitializing the ProseMirror editor on changes (content, styles, etc.) may reset selection unexpectedly. Verify if this is intended. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify if the behavior of resetting the selection is intended when reinitializing the ProseMirror editor. This falls under asking the author to confirm their intention, which is against the rules.
10. apps/web/src/app/project/[id]/_components/canvas/index.tsx:47
- Draft comment:
Typographical error: The variable 'lintedScale' (line 47) likely should be renamed to 'clampedScale' to better reflect its purpose after applying clampZoom. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_CNbgp91YYXTg1tTn
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/project/[id]/_components/canvas/overlay/elements/measurement.tsx
Show resolved
Hide resolved
apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx
Show resolved
Hide resolved
apps/web/src/app/project/[id]/_components/canvas/overlay/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 7e8e1ed in 1 minute and 56 seconds
More details
- Looked at
87
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
9
drafted comments based on config settings.
1. apps/studio/src/routes/editor/Canvas/Overlay/Chat.tsx:57
- Draft comment:
prevChatPositionRef is updated but not used anywhere. Consider removing it to clean up the code. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. apps/studio/src/routes/editor/Canvas/Overlay/Chat.tsx:85
- Draft comment:
Removal of distance-based animation condition is intentional? Verify that a static animationClass meets the design requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%
<= threshold50%
The comment asks the PR author to verify if a staticanimationClass
meets the design requirements, which is against the rules. However, it also questions if the removal of a condition is intentional, which is allowed. The comment is partially useful but needs to be rephrased to comply with the rules.
3. apps/studio/src/routes/editor/Canvas/Overlay/MeasurementOverlay.tsx:405
- Draft comment:
Removed duplicate comment for 'Distance lines and labels' cleans up the code—good job. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. apps/web/src/app/project/[id]/_components/canvas/overlay/index.tsx:65
- Draft comment:
Formatting fix: Changed '< div' to '<div' improves consistency. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. apps/web/src/app/project/[id]/_components/canvas/overlay/index.tsx:98
- Draft comment:
PR description mentions onboarding docs improvements, but changes here focus on UI overlays. Verify that the documentation updates are included elsewhere or planned. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. apps/studio/src/routes/editor/Canvas/Overlay/MeasurementOverlay.tsx:405
- Draft comment:
Removed duplicate comment ('Distance lines and labels') to clean up the code. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/measurement.tsx:404
- Draft comment:
Removed duplicate comment ('Distance lines and labels') to reduce redundancy. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. apps/web/src/app/project/[id]/_components/canvas/overlay/index.tsx:64
- Draft comment:
Minor formatting fix: removed the extra space in thetag. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
9. apps/web/src/app/project/[id]/_components/canvas/overlay/index.tsx:62
- Draft comment:
The PR description mentions onboarding documentation improvements (e.g. for Next.js, Remix, Webpack, Babel), but no documentation changes are present. Please ensure that the framework docs updates are included or update the PR description accordingly. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_D8hXCdQgsAgAwQXk
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Math.pow(chatPosition.y - prevChatPositionRef.current.y, 2), | ||
) | ||
: 0; | ||
|
||
useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prevChatPositionRef is still updated but no longer used; consider removing it if not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 7cd6dac in 1 minute and 8 seconds
More details
- Looked at
41
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx:72
- Draft comment:
Good cleanup: removed distance calculation since both branches returned the same string. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx:55
- Draft comment:
Removed redundant comments help clarity. The reset input effect on elementId change is maintained. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx:67
- Draft comment:
Removed distance calculation and conditional animationClass since both branches were identical. Good cleanup. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining what was done in the code without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements, so it should be removed according to the rules.
4. apps/web/src/app/project/[id]/_components/canvas/overlay/elements/chat.tsx:1
- Draft comment:
PR description mentions onboarding docs ([FEAT] Add better onboarding docs #123), but this diff only refactors chat overlay logic. Ensure onboarding docs changes are included elsewhere if required. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_JZiFcPGcNG1aE34R
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 3836b4c in 1 minute and 3 seconds
More details
- Looked at
14
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. apps/studio/src/routes/editor/Canvas/Overlay/MeasurementOverlay.tsx:2
- Draft comment:
Good use of an absolute import for RectDimensions. This update improves maintainability compared to the relative path import. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. apps/studio/src/routes/editor/Canvas/Overlay/MeasurementOverlay.tsx:2
- Draft comment:
Good change: using an absolute import for RectDimensions improves maintainability and reduces fragility with relative paths. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_6cASrvfnXVLZecWg
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
* Continue adding web * Clean unnecessary callbacks * restore studio * Add layout * Update providers * More state working * Add more states * Working canvas state * Projects select * Add resize handles * Fix typo * Add user states
* Continue adding web * Clean unnecessary callbacks * restore studio * Add layout * Update providers * More state working * Add more states * Working canvas state * Projects select * Add resize handles * Fix typo * Add user states
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
This PR introduces a new web version with enhanced components, UI updates, and refactoring, including new
Frame
andOverlay
components, icon updates, and improved toast management.Frame
,Frames
, andResizeHandles
components inapps/web/src/app/project/[id]/_components/canvas/frames/
.OverlayChat
,MeasurementOverlay
, and variousRect
components inapps/web/src/app/project/[id]/_components/canvas/overlay/elements/
.Canvas
andOverlay
components inapps/web/src/app/project/[id]/_components/canvas/
.Orientation.Potrait
toOrientation.Portrait
inpackages/models/src/constants/editor.ts
and updated related components.assertNever
topackages/utility/src/assert.ts
and exported it inpackages/utility/src/index.ts
.Icons
inpackages/ui-v4/src/components/icons/index.tsx
andpackages/ui/src/components/icons/index.tsx
to includePortrait
andLandscape
icons.use-toast.tsx
to improve toast management with a new reducer and action types..env.example
to useNEXT_PUBLIC_SUPABASE_URL
andNEXT_PUBLIC_SUPABASE_ANON_KEY
.sendAnalytics
function inapps/web/src/utils/analytics/index.ts
.This description was created by
for 3836b4c. It will automatically update as commits are pushed.