Open
Conversation
- Allow local developers to override ports when they conflict. Defaults set so will not hinder experience or existing deployments
…efaults set so will not hinder experience or existing deployments
|
|
Contributor
📝 WalkthroughWalkthroughSwitched a Dockerfile VOLUME quoting style; replaced hardcoded host ports in docker-compose with env-var overrides and defaults; updated web dev script to use an env-var-backed port; added accessibility/focus attributes and minor typings/handler tweaks to an editor image block component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… 'HTMLDivElement | null'. Fixed by Typcast. Fixed warnings.
Typescript error fix:
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
338-354:⚠️ Potential issue | 🟡 MinorFocusable resize handle lacks keyboard interaction.
With
tabIndex={0}, this element appears in the tab order, but keyboard users cannot operate it since there are noonKeyDownhandlers for resizing. This creates a confusing experience where the element is announced as a button but provides no keyboard interaction.Consider one of these approaches:
- Add keyboard support (e.g., arrow keys to resize incrementally).
- Use
tabIndex={-1}if keyboard resizing isn't intended (keeps it out of tab order).- Use
role="slider"witharia-valuenow,aria-valuemin,aria-valuemaxfor proper resize semantics.💡 Option 2: Remove from tab order if keyboard resizing isn't supported
role="button" aria-roledescription="Resize image" - tabIndex={0} + tabIndex={-1} onMouseDown={handleResizeStart} onTouchStart={handleResizeStart}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/core/extensions/custom-image/components/block.tsx` around lines 338 - 354, The resize handle is focusable (tabIndex={0}) but lacks keyboard handlers; either remove it from the tab order or add proper keyboard support. Add an onKeyDown handler (e.g., handleResizeKeyDown or extend handleResizeStart) on the same element to handle ArrowLeft/ArrowRight/ArrowUp/ArrowDown (and Enter/Space if needed) to perform incremental resizing using the same resize logic, and update ARIA semantics accordingly (keep role="button" and add aria-label, or change to role="slider" and provide aria-valuemin/aria-valuemax/aria-valuenow if exposing value). Alternatively, if keyboard resizing is not supported, change tabIndex to -1 to remove it from the tab order. Ensure references to isResizing and nodeAlignment remain unchanged.
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
238-248: Consider keyboard support or alternative role.Adding
role="button"implies keyboard interactivity (Enter/Space activation). WhiletabIndex={-1}keeps this out of the tab order, screen readers may still announce it as a button. Consider either:
- Adding an
onKeyDownhandler for Enter/Space to match theonMouseDownbehavior, or- Using a more appropriate role (e.g.,
role="img"witharia-label, or no explicit role since this is primarily for selection).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/core/extensions/custom-image/components/block.tsx` around lines 238 - 248, The element with ref containerRef and className "group/image-component" currently uses role="button" but tabIndex={-1} without keyboard handlers; either add an onKeyDown handler that listens for Enter and Space and delegates to handleImageMouseDown (so keyboard activation matches mouse behavior and preserves role="button"), or change the role to a non-interactive/semantic alternative (e.g., role="img") and add an appropriate aria-label or remove the explicit role so screen readers treat it as an image/selection-only element; update the JSX for the element using the chosen approach and ensure handleImageMouseDown is invoked for keyboard activation if you keep role="button".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/editor/src/core/extensions/custom-image/components/block.tsx`:
- Around line 338-354: The resize handle is focusable (tabIndex={0}) but lacks
keyboard handlers; either remove it from the tab order or add proper keyboard
support. Add an onKeyDown handler (e.g., handleResizeKeyDown or extend
handleResizeStart) on the same element to handle
ArrowLeft/ArrowRight/ArrowUp/ArrowDown (and Enter/Space if needed) to perform
incremental resizing using the same resize logic, and update ARIA semantics
accordingly (keep role="button" and add aria-label, or change to role="slider"
and provide aria-valuemin/aria-valuemax/aria-valuenow if exposing value).
Alternatively, if keyboard resizing is not supported, change tabIndex to -1 to
remove it from the tab order. Ensure references to isResizing and nodeAlignment
remain unchanged.
---
Nitpick comments:
In `@packages/editor/src/core/extensions/custom-image/components/block.tsx`:
- Around line 238-248: The element with ref containerRef and className
"group/image-component" currently uses role="button" but tabIndex={-1} without
keyboard handlers; either add an onKeyDown handler that listens for Enter and
Space and delegates to handleImageMouseDown (so keyboard activation matches
mouse behavior and preserves role="button"), or change the role to a
non-interactive/semantic alternative (e.g., role="img") and add an appropriate
aria-label or remove the explicit role so screen readers treat it as an
image/selection-only element; update the JSX for the element using the chosen
approach and ensure handleImageMouseDown is invoked for keyboard activation if
you keep role="button".
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Type of Change
Summary by CodeRabbit
Chores
Bug Fixes