Skip to content

feat(alpha-editor): update tooltip icons and layout in alpha editor #1193

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.

Sign up for GitHub

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

Closed
dineshsutihar wants to merge 2 commits into from
Closed

feat(alpha-editor): update tooltip icons and layout in alpha editor #1193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21e22d4
feat: update tooltip icons and layout in alpha editor
dineshsutihar Oct 30, 2024
9b5b1cf
refactor: Update bubble-menu.tsx component
dineshsutihar Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 45 additions & 30 deletions components/editor/editor/components/bubble-menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import {
BoldIcon,
ItalicIcon,
CodeIcon,
QuoteIcon,
MinusSquareIcon,
Type as TextIcon,
Heading,
} from "lucide-react";

import { NodeSelector } from "./node-selector";
import { LinkSelector } from "./link-selector";
import { cn } from "@/utils/utils";

Expand All @@ -23,34 +25,56 @@ type EditorBubbleMenuProps = Omit<Required<Pick<BubbleMenuProps, "editor">> & Bu

export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
const items: BubbleMenuItem[] = [
{
name: "heading",
isActive: () => props.editor?.isActive("heading", { level: 2 }) ?? false,
command: () => props.editor?.chain().focus().toggleHeading({ level: 2 }).run(),
icon: Heading,
},
{
name: "text",
isActive: () => props.editor?.isActive("paragraph") ?? false,
command: () => props.editor?.chain().focus().setParagraph().run(),
icon: TextIcon,
},
{
name: "bold",
isActive: () => props.editor.isActive("bold"),
command: () => props.editor.chain().focus().toggleBold().run(),
isActive: () => props.editor?.isActive("bold") ?? false,
command: () => props.editor?.chain().focus().toggleBold().run(),
Comment on lines +42 to +43
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Error handling is inconsistent across editor operations

The search results reveal inconsistencies in error handling:

  • In bubble-menu.tsx, all props.editor accesses use optional chaining (?.) and null coalescing (??)
  • However, in other files:
    • slash-command.tsx uses direct access: editor: props.editor
    • node-selector.tsx uses direct editor access without optional chaining in its commands
    • link-selector.tsx also uses direct editor access without safeguards

These inconsistencies could lead to runtime errors if the editor instance is undefined.

🔗 Analysis chain

Verify error handling consistency across all editor operations.

While the optional chaining is correctly implemented for these operations, let's ensure it's consistently applied throughout the codebase.

Also applies to: 48-49, 54-55

The previous command failed because the file type was not recognized. Let's try with explicit file extensions and search for both the editor access patterns and chain operations to verify the consistency of error handling.

Let's try without file type flags since they're not recognized, and also look for the specific file we're reviewing to understand its location and potentially related files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for direct editor access without optional chaining
rg "props\.editor\." --type typescript

Length of output: 75

Script:

#!/bin/bash
# Search for editor access patterns in TypeScript/TSX files
rg "props\.editor" -t tsx -t ts

# Also search for chain().focus() patterns to find editor operations
rg "chain\(\)\.focus\(\)" -t tsx -t ts

Length of output: 130

Script:

#!/bin/bash
# Search for editor access patterns without file type flags
rg "props\.editor"

# Search for chain().focus() patterns
rg "chain\(\)\.focus\(\)"

# List all files that might contain editor operations
fd "bubble-menu" --extension tsx --extension ts

Length of output: 4878

Copy link
Contributor Author

@dineshsutihar dineshsutihar Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the error handling in bubble-menu.tsx to use optional chaining and null coalescing as suggested, since TypeScript was indicating that props.editor could potentially be null. This change provides better type safety while maintaining the component's functionality.

Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dineshsutihar, great job updating the error handling in bubble-menu.tsx with optional chaining and null coalescing. This enhances type safety and code robustness.

There are similar instances in other files (slash-command.tsx, node-selector.tsx, link-selector.tsx) where props.editor is accessed directly without optional chaining. Updating these would further improve consistency and prevent potential runtime errors.

(^_^)

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

icon: BoldIcon,
},
{
name: "italic",
isActive: () => props.editor.isActive("italic"),
command: () => props.editor.chain().focus().toggleItalic().run(),
isActive: () => props.editor?.isActive("italic") ?? false,
command: () => props.editor?.chain().focus().toggleItalic().run(),
icon: ItalicIcon,
},
{
name: "code",
isActive: () => props.editor.isActive("code"),
command: () => props.editor.chain().focus().toggleCode().run(),
isActive: () => props.editor?.isActive("code") ?? false,
command: () => props.editor?.chain().focus().toggleCode().run(),
icon: CodeIcon,
},
{
name: "quote",
isActive: () => props.editor?.isActive("blockquote") ?? false,
command: () => props.editor?.chain().focus().toggleBlockquote().run(),
icon: QuoteIcon,
},
{
name: "horizontalRule",
isActive: () => false,
command: () => props.editor?.chain().focus().setHorizontalRule().run(),
icon: MinusSquareIcon,
},
];

const bubbleMenuProps: EditorBubbleMenuProps = {
...props,
shouldShow: ({ editor }) => {
// don't show if image is selected or a heading
if (editor.isActive("image") || editor.isActive("heading")) {
if (editor.isActive("image")) {
return false;
}

try {
if (editor.view.state.selection.$from.before() === 0) {
return false;
Expand All @@ -65,36 +89,18 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
tippyOptions: {
moveTransition: "transform 0.15s ease-out",
onHidden: () => {
setIsNodeSelectorOpen(false);
setIsLinkSelectorOpen(false);
},
},
};

const [isNodeSelectorOpen, setIsNodeSelectorOpen] = useState(false);
const [isLinkSelectorOpen, setIsLinkSelectorOpen] = useState(false);

return (
<BubbleMenu
{...bubbleMenuProps}
className="flex w-fit divide-x divide-stone-200 rounded border border-stone-200 bg-white shadow-xl"
>
<NodeSelector
editor={props.editor}
isOpen={isNodeSelectorOpen}
setIsOpen={() => {
setIsNodeSelectorOpen(!isNodeSelectorOpen);
setIsLinkSelectorOpen(false);
}}
/>
<LinkSelector
editor={props.editor}
isOpen={isLinkSelectorOpen}
setIsOpen={() => {
setIsLinkSelectorOpen(!isLinkSelectorOpen);
setIsNodeSelectorOpen(false);
}}
/>
<div className="flex">
{items.map((item, index) => (
<button
Expand All @@ -111,6 +117,15 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
</button>
))}
</div>
{props.editor && (
<LinkSelector
editor={props.editor}
isOpen={isLinkSelectorOpen}
setIsOpen={() => {
setIsLinkSelectorOpen(!isLinkSelectorOpen);
}}
/>
)}
</BubbleMenu>
);
};
};
13 changes: 5 additions & 8 deletions components/editor/editor/components/link-selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { cn, getUrlFromString } from "@/utils/utils";
import type { Editor } from "@tiptap/core";
import { Check, Trash } from "lucide-react";
import { Link2Icon } from "lucide-react";
import type { Dispatch, FC, SetStateAction } from "react";
import { useEffect, useRef, useCallback } from "react";

Expand Down Expand Up @@ -60,14 +60,11 @@ export const LinkSelector: FC<LinkSelectorProps> = ({
className="flex h-full items-center space-x-2 px-3 py-1.5 text-sm font-medium text-stone-600 hover:bg-stone-100 active:bg-stone-200"
onClick={setLink}
>
<p className="text-base">↗</p>
<p
className={cn("underline decoration-stone-400 underline-offset-4", {
<Link2Icon
className={cn("h-4 w-4", {
"text-blue-500": editor.isActive("link"),
})}
>
Link
</p>
/>
</button>
{/* {isOpen && (
<form
Expand Down Expand Up @@ -106,4 +103,4 @@ export const LinkSelector: FC<LinkSelectorProps> = ({
)} */}
</div>
);
};
};