[ENG-2088] Add tag to block button on canvas block cards #1351
+172
−101
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ import { getRelationColor } from "./DiscourseRelationShape/DiscourseRelationUtil | |
| import { getPersonalSetting } from "~/components/settings/utils/accessors"; | ||
| import { PERSONAL_KEYS } from "~/components/settings/utils/settingKeys"; | ||
| import DiscourseContextOverlay from "~/components/DiscourseContextOverlay"; | ||
| import NodeMenu from "~/components/DiscourseNodeMenu"; | ||
| import { getDiscourseNodeColors } from "~/utils/getDiscourseNodeColors"; | ||
| import { render as renderToast } from "roamjs-components/components/Toast"; | ||
| import { RenderRoamBlockString } from "~/utils/roamReactComponents"; | ||
|
|
@@ -459,6 +460,8 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil<DiscourseNodeShape> { | |
| // eslint-disable-next-line react-hooks/rules-of-hooks | ||
| const [overlayMounted, setOverlayMounted] = useState(false); | ||
| // eslint-disable-next-line react-hooks/rules-of-hooks | ||
| const [addTagMenuKey, setAddTagMenuKey] = useState(0); | ||
| // eslint-disable-next-line react-hooks/rules-of-hooks | ||
| const dialogRenderedRef = useRef(false); | ||
|
|
||
| // Detect discourse node tags in block text for blck-node shapes | ||
|
|
@@ -487,6 +490,32 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil<DiscourseNodeShape> { | |
| return null; | ||
| }, [shape]); | ||
|
|
||
| const showAddTagButton = | ||
| getDiscourseNodeTypeId({ shape }) === "blck-node" && | ||
| isLiveBlock(shape.props.uid) && | ||
| Object.values(discourseContext.nodes).some( | ||
| (n) => n.backedBy === "user" && n.tag, | ||
| ); | ||
|
|
||
| const handleTagAdded = (newText: string) => { | ||
| const updateShape = async () => { | ||
| if (!extensionAPI) return; | ||
| const { h, w, imageUrl } = await calcCanvasNodeSizeAndImg({ | ||
| nodeText: newText, | ||
| uid: shape.props.uid, | ||
| nodeType: getDiscourseNodeTypeId({ shape }), | ||
| extensionAPI, | ||
| }); | ||
| this.updateProps(shape.id, shape.type, { | ||
| title: newText, | ||
| h, | ||
| w, | ||
| imageUrl, | ||
| }); | ||
| }; | ||
| void updateShape(); | ||
| }; | ||
|
|
||
| const { backgroundColor, textColor } = this.getColors(shape); | ||
| const showEmbeddedRoamBlock = | ||
| !isPageUid(shape.props.uid) && isLiveBlock(shape.props.uid); | ||
|
|
@@ -619,114 +648,145 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil<DiscourseNodeShape> { | |
| className="relative flex h-full min-h-0 w-full min-w-0 flex-col" | ||
| style={{ pointerEvents: "all" }} | ||
| > | ||
| <div className="absolute left-1 top-1 z-10 flex items-center"> | ||
| {/* Open in Sidebar Button */} | ||
| <Button | ||
| minimal | ||
| small | ||
| icon={ | ||
| <Icon | ||
| icon="panel-stats" | ||
| color={textColor} | ||
| className="opacity-50" | ||
| /> | ||
| } | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| void openBlockInSidebar(shape.props.uid); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| title="Open in sidebar (Shift+Click)" | ||
| /> | ||
|
|
||
| {/* Add Tag to Block Button */} | ||
| {extensionAPI && showAddTagButton && ( | ||
| <NodeMenu | ||
| key={addTagMenuKey} | ||
| blockUid={shape.props.uid} | ||
| extensionAPI={extensionAPI} | ||
| onClose={() => setAddTagMenuKey((k) => k + 1)} | ||
| onTagAdded={handleTagAdded} | ||
|
Comment on lines
+673
to
+678
There was a problem hiding this comment.
Add meaningful unit tests covering tag selection, the asynchronous block update, and the subsequent shape title/size refresh; this commit introduces that behavior without any tests, so regressions such as a rejected update or stale shape state will not be detected. AGENTS.md reference: AGENTS.md:L82-L85 Useful? React with 👍 / 👎. |
||
| trigger={ | ||
| <Button | ||
| minimal | ||
| small | ||
| icon={ | ||
| <span className="opacity-50" style={{ color: textColor }}> | ||
| # | ||
| </span> | ||
| } | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| title="Add tag" | ||
| > | ||
| <span | ||
| className="opacity-70" | ||
| style={{ color: textColor, fontSize: "11px" }} | ||
| > | ||
| Add tag | ||
| </span> | ||
| </Button> | ||
| } | ||
| /> | ||
| )} | ||
|
|
||
| {/* Convert to Node Type Button */} | ||
| {matchedNodeForConversion && ( | ||
| <Button | ||
| minimal | ||
| small | ||
| icon={ | ||
| <Icon icon="plus" color={textColor} className="opacity-50" /> | ||
| } | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| const { node, blockText } = matchedNodeForConversion; | ||
| const tag = node.tag; | ||
| if (!tag) return; | ||
| const cleanTag = getCleanTagText(tag); | ||
| const escapedCleanTag = escapeRegExp(cleanTag); | ||
| // Strip the tag from block text (same pattern as detection above) | ||
| const cleanedText = blockText | ||
| .replace( | ||
| new RegExp(`#\\[\\[${escapedCleanTag}\\]\\]`, "i"), | ||
| "", | ||
| ) | ||
| .replace(new RegExp(`#${escapedCleanTag}`, "i"), "") | ||
| .trim(); | ||
| const { x, y } = shape; | ||
| renderModifyNodeDialog({ | ||
| mode: "create", | ||
| nodeType: node.type, | ||
| initialValue: { text: cleanedText, uid: "" }, | ||
| extensionAPI, | ||
| includeDefaultNodes: true, | ||
| disableNodeTypeChange: true, | ||
| onSuccess: async ({ text, uid }) => { | ||
| if (!extensionAPI) return; | ||
| try { | ||
| const { | ||
| h, | ||
| w, | ||
| imageUrl: nodeImageUrl, | ||
| } = await calcCanvasNodeSizeAndImg({ | ||
| nodeText: text, | ||
| extensionAPI, | ||
| nodeType: node.type, | ||
| uid, | ||
| }); | ||
| editor.createShapes([ | ||
| { | ||
| type: DISCOURSE_NODE_SHAPE_TYPE, | ||
| id: createShapeId(), | ||
| props: { | ||
| uid, | ||
| title: text, | ||
| h, | ||
| w, | ||
| imageUrl: nodeImageUrl, | ||
| fontFamily: "sans", | ||
| size: "s", | ||
| nodeTypeId: node.type, | ||
| }, | ||
| x, | ||
| y, | ||
| }, | ||
| ]); | ||
| editor.deleteShapes([shape.id]); | ||
| } catch (error) { | ||
| renderToast({ | ||
| id: `discourse-node-convert-error-${Date.now()}`, | ||
| intent: "danger", | ||
| content: ( | ||
| <span>Error converting block: {String(error)}</span> | ||
| ), | ||
| }); | ||
| } | ||
| }, | ||
| onClose: () => {}, | ||
| }); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| title={`Convert to ${matchedNodeForConversion.node.text}`} | ||
| > | ||
| <span | ||
| className="opacity-70" | ||
| style={{ color: textColor, fontSize: "11px" }} | ||
| > | ||
| Convert to {matchedNodeForConversion.node.text} | ||
| </span> | ||
| </Button> | ||
| )} | ||
| </div> | ||
|
|
||
| {shape.props.imageUrl && isKeyImage === "true" ? ( | ||
| <div className="mt-2 flex min-h-0 w-full flex-1 items-center justify-center overflow-hidden"> | ||
|
|
||
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.
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.
On canvases with many live block cards, this mounts a separate
NodeMenufor every card even while all menus are closed. Each instance reads and parses all discourse nodes and, because it has a trigger, registers its ownkeydownandkeyuplisteners ondocument; consequently initialization and every keyboard event scale with the number of mounted cards. Lazily mounting the menu after the trigger is activated, or sharing one menu among cards, avoids that canvas-wide overhead.Useful? React with 👍 / 👎.