refactor: to new canvas impl instead of DOM #104
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
aidenybai
changed the title
- Enhanced the `getGrabbedBoxInfo` and `isGrabbedBoxVisible` functions to retrieve grabbed box information from the API state, improving accuracy in box visibility checks. - Updated type definitions in `types.ts` to ensure proper structure for `grabbedBoxes`, including `id`, `bounds`, and `createdAt` properties. - Refactored core component logic to utilize the updated grabbed boxes structure, ensuring consistent handling across the application.
- Changed the global name for the ReactGrab module to enhance compatibility with different environments. - Refactored various functions in the API to ensure they correctly access and utilize the updated global state, improving overall functionality and reliability. - Enhanced type definitions to better reflect the structure of the API state, ensuring consistency across the application.
- Updated the target element check in the API to use a more concise null check, enhancing code readability. - Ensured consistent handling of the canvas element retrieval, maintaining functionality across different environments.
- Enhanced the handling of device pixel ratio changes by adding event listeners for better responsiveness to window resizing and pixel ratio adjustments. - Refactored the setup and cleanup logic for media queries to ensure proper resource management and prevent memory leaks.
- Updated overlay canvas component to utilize constants for color and style definitions, improving maintainability and consistency. - Refactored the handling of crosshair and overlay styles to enhance visual clarity and responsiveness. - Adjusted type definitions in fixtures to ensure alignment with updated component logic.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- Updated the `waitForSelectionLabel` and `isSelectionBoxVisible` functions to improve their responsiveness and accuracy in detecting visibility states. - Refactored the handling of selection label information retrieval to ensure consistent behavior across different components. - Adjusted type definitions in `types.ts` to align with the updated logic for selection visibility, enhancing overall code clarity and maintainability.
- Updated the logic for determining the visibility of selection and drag boxes to improve accuracy and responsiveness. - Refactored the handling of visibility conditions to incorporate additional state checks, ensuring consistent behavior across components.
- Updated the input mode tests to improve the accuracy of clipboard content checks and input focus behavior. - Refactored selection visibility checks to ensure consistent behavior when interacting with elements. - Enhanced toolbar visibility tests to better handle viewport changes and rapid resizing scenarios.
- Updated the overlay canvas component to improve the handling of animated bounds, including target opacity and border radius. - Refactored the interpolation logic to ensure smoother transitions and better responsiveness to animation state changes. - Adjusted type definitions for animation properties to enhance clarity and maintainability.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/react-grab/src/components/overlay-canvas.tsx">
<violation number="1" location="packages/react-grab/src/components/overlay-canvas.tsx:537">
P2: Magic number `0.01` used for opacity convergence threshold in two places. Consider extracting to a constant (e.g., `OPACITY_CONVERGENCE_THRESHOLD`) to keep both usages in sync.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
3 issues found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/react-grab/e2e/open-file.spec.ts">
<violation number="1" location="packages/react-grab/e2e/open-file.spec.ts:38">
P2: The `pressKeyCombo` action is being called inside the `expect.poll()` callback, which means it will be pressed repeatedly every 500ms until the condition passes. This masks timing issues and can cause flaky tests. The action should be performed once before the poll, then poll only the result. This pattern is repeated multiple times in this file.</violation>
<violation number="2" location="packages/react-grab/e2e/open-file.spec.ts:396">
P2: Loosening the assertion from `.toBe(1)` to `.toBeGreaterThanOrEqual(1)` (and `.toBe(2)` to `.toBeGreaterThanOrEqual(2)`) works around the repeated key press issue but makes the test less precise. Once the key press is moved outside the poll, the exact count assertions should be restored.</violation>
</file>
<file name="packages/react-grab/e2e/fixtures.ts">
<violation number="1" location="packages/react-grab/e2e/fixtures.ts:297">
P2: The strict inequality check `!== null` will incorrectly pass when `selectionFilePath` is `undefined` (since `undefined !== null` is `true`). This can cause the function to resolve prematurely when the API isn't ready or the property hasn't been set. Use loose equality `!= null` instead to catch both `null` and `undefined`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
Migrates overlay rendering to a single high-DPI
OverlayCanvaswith offscreen layers and lerped animations, reducing DOM churn and centralizing drawing.SelectionBox/CrosshairwithOverlayCanvas(layers: crosshair/drag/selection/grabbed/processing); renderer now mounts the canvas and retainsSelectionLabelgetStateand plugin hooks withisCrosshairVisible,isSelectionBoxVisible,isDragBoxVisible,grabbedBoxes,selectionFilePathwaitForActive,waitForSelectionBox/Label/Source, context-menu waiters); specs updated to read fromwindow.__REACT_GRAB__globalThis.__REACT_GRAB_MODULE__Written by Cursor Bugbot for commit 72708a8. This will update automatically on new commits. Configure here.
Summary by cubic
Switched to a single canvas-based overlay for crosshair, selection, drag, and grabbed states to reduce DOM churn and improve performance. Updated renderer and E2E helpers to use the new overlay and internal state.
Refactors
Migration
Written for commit 72708a8. Summary will update on new commits.