-
Notifications
You must be signed in to change notification settings - Fork 4
[ENG-1036] Fix the relation color not picking by forcing colors to strictly match Tldraw palette #557
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
base: main
Are you sure you want to change the base?
[ENG-1036] Fix the relation color not picking by forcing colors to strictly match Tldraw palette #557
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,112 @@ | ||
| import { useState } from "react"; | ||
| import { useState, useRef, useEffect } from "react"; | ||
| import { DiscourseRelationType } from "~/types"; | ||
| import { Notice } from "obsidian"; | ||
| import { usePlugin } from "./PluginContext"; | ||
| import generateUid from "~/utils/generateUid"; | ||
| import { ConfirmationModal } from "./ConfirmationModal"; | ||
| import { | ||
| TLDRAW_COLOR_NAMES, | ||
| TLDRAW_COLOR_LABELS, | ||
| DEFAULT_TLDRAW_COLOR, | ||
| COLOR_PALETTE, | ||
| type TldrawColorName, | ||
| } from "~/utils/tldrawColors"; | ||
| import { getContrastColor } from "~/utils/colorUtils"; | ||
|
|
||
| type ColorPickerProps = { | ||
| value: string; | ||
| onChange: (color: TldrawColorName) => void; | ||
| }; | ||
|
|
||
| const ColorPicker = ({ value, onChange }: ColorPickerProps) => { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const dropdownRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| useEffect(() => { | ||
| const handleClickOutside = (event: MouseEvent) => { | ||
| if ( | ||
| dropdownRef.current && | ||
| !dropdownRef.current.contains(event.target as Node) | ||
| ) { | ||
| setIsOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| if (isOpen) { | ||
| document.addEventListener("mousedown", handleClickOutside); | ||
| return () => { | ||
| document.removeEventListener("mousedown", handleClickOutside); | ||
| }; | ||
| } | ||
| }, [isOpen]); | ||
|
|
||
| const currentColor = value as TldrawColorName; | ||
| const bgColor = COLOR_PALETTE[currentColor] ?? COLOR_PALETTE.black; | ||
| const textColor = getContrastColor(bgColor ?? DEFAULT_TLDRAW_COLOR); | ||
|
|
||
| return ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remember to use tailwind classes instead of inline styles |
||
| <div ref={dropdownRef} className="relative" style={{ minWidth: "140px" }}> | ||
| <button | ||
| type="button" | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| className="flex w-full items-center justify-between rounded border px-3 py-2 text-left" | ||
| style={{ | ||
| backgroundColor: bgColor, | ||
| color: textColor, | ||
| }} | ||
| > | ||
| <span className="flex items-center gap-2"> | ||
| <span | ||
| className="inline-block rounded-full" | ||
| style={{ | ||
| width: "12px", | ||
| height: "12px", | ||
| backgroundColor: bgColor, | ||
| border: `2px solid ${textColor}`, | ||
| }} | ||
| /> | ||
| {TLDRAW_COLOR_LABELS[currentColor]} | ||
| </span> | ||
| <span style={{ fontSize: "10px" }}>{isOpen ? "▲" : "▼"}</span> | ||
| </button> | ||
|
|
||
| {isOpen && ( | ||
| <div | ||
| className="absolute z-50 mt-1 w-full" | ||
| style={{ | ||
| maxHeight: "300px", | ||
| overflowY: "auto", | ||
| }} | ||
| > | ||
| {TLDRAW_COLOR_NAMES.map((colorName) => { | ||
| const bgColor = COLOR_PALETTE[colorName] ?? COLOR_PALETTE.black; | ||
| return ( | ||
| <button | ||
| key={colorName} | ||
| type="button" | ||
| onClick={() => { | ||
| onChange(colorName); | ||
| setIsOpen(false); | ||
| }} | ||
| className="flex w-full flex-row justify-start gap-2 rounded-none px-3 py-2" | ||
| > | ||
| <span | ||
| className="inline-block rounded-full" | ||
| style={{ | ||
| width: "12px", | ||
| height: "12px", | ||
| backgroundColor: bgColor, | ||
| }} | ||
| /> | ||
| {TLDRAW_COLOR_LABELS[colorName]} | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| const RelationshipTypeSettings = () => { | ||
| const plugin = usePlugin(); | ||
|
|
@@ -24,11 +127,14 @@ const RelationshipTypeSettings = () => { | |
| id: newId, | ||
| label: "", | ||
| complement: "", | ||
| color: "#000000", | ||
| color: DEFAULT_TLDRAW_COLOR, | ||
| }; | ||
| } | ||
|
|
||
| updatedRelationTypes[index][field] = value; | ||
| if (field === "color") { | ||
| updatedRelationTypes[index].color = value as TldrawColorName; | ||
| } else { | ||
| updatedRelationTypes[index][field] = value; | ||
| } | ||
| setRelationTypes(updatedRelationTypes); | ||
| setHasUnsavedChanges(true); | ||
| }; | ||
|
|
@@ -42,7 +148,7 @@ const RelationshipTypeSettings = () => { | |
| id: newId, | ||
| label: "", | ||
| complement: "", | ||
| color: "#000000", | ||
| color: DEFAULT_TLDRAW_COLOR, | ||
| }, | ||
| ]; | ||
| setRelationTypes(updatedRelationTypes); | ||
|
|
@@ -53,7 +159,7 @@ const RelationshipTypeSettings = () => { | |
| const relationType = relationTypes[index] || { | ||
| label: "Unnamed", | ||
| complement: "", | ||
| color: "#000000", | ||
| color: DEFAULT_TLDRAW_COLOR, | ||
| }; | ||
| const modal = new ConfirmationModal(plugin.app, { | ||
| title: "Delete Relation Type", | ||
|
|
@@ -84,7 +190,7 @@ const RelationshipTypeSettings = () => { | |
|
|
||
| const handleSave = async (): Promise<void> => { | ||
| for (const relType of relationTypes) { | ||
| if (!relType.id || !relType.label || !relType.complement || !relType.color) { | ||
| if (!relType.id || !relType.label || !relType.complement) { | ||
| new Notice("All fields are required for relation types."); | ||
| return; | ||
| } | ||
|
|
@@ -132,14 +238,11 @@ const RelationshipTypeSettings = () => { | |
| } | ||
| className="flex-1" | ||
| /> | ||
| <input | ||
| type="color" | ||
| <ColorPicker | ||
| value={relationType.color} | ||
| onChange={(e) => | ||
| handleRelationTypeChange(index, "color", e.target.value) | ||
| onChange={(color) => | ||
| handleRelationTypeChange(index, "color", color) | ||
| } | ||
| className="w-12 h-8 rounded border" | ||
| title="Relation color" | ||
| /> | ||
| <button | ||
| onClick={() => confirmDeleteRelationType(index)} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /** | ||
| * Tldraw color names that can be used for relation types. | ||
| * These match the defaultColorNames from tldraw's TLColorStyle. | ||
| */ | ||
| export const TLDRAW_COLOR_NAMES = [ | ||
| "black", | ||
| "grey", | ||
| "light-violet", | ||
| "violet", | ||
| "blue", | ||
| "light-blue", | ||
| "yellow", | ||
| "orange", | ||
| "green", | ||
| "light-green", | ||
| "light-red", | ||
| "red", | ||
| "white", | ||
| ] as const; | ||
|
|
||
| export type TldrawColorName = (typeof TLDRAW_COLOR_NAMES)[number]; | ||
|
|
||
| /** | ||
| * Human-readable labels for tldraw color names | ||
| */ | ||
| export const TLDRAW_COLOR_LABELS: Record<TldrawColorName, string> = { | ||
| black: "Black", | ||
| grey: "Grey", | ||
| "light-violet": "Light Violet", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add ignore for: Object Literal Property name |
||
| violet: "Violet", | ||
| blue: "Blue", | ||
| "light-blue": "Light Blue", | ||
| yellow: "Yellow", | ||
| orange: "Orange", | ||
| green: "Green", | ||
| "light-green": "Light Green", | ||
| "light-red": "Light Red", | ||
| red: "Red", | ||
| white: "White", | ||
| }; | ||
|
|
||
| export const DEFAULT_TLDRAW_COLOR: TldrawColorName = "black"; | ||
|
|
||
| // from @tldraw/editor/editor.css | ||
| /* eslint-disable @typescript-eslint/naming-convention */ | ||
| export const COLOR_PALETTE: Record<string, string> = { | ||
| black: "#1d1d1d", | ||
| blue: "#4263eb", | ||
| green: "#099268", | ||
| grey: "#adb5bd", | ||
| "light-blue": "#4dabf7", | ||
| "light-green": "#40c057", | ||
| "light-red": "#ff8787", | ||
| "light-violet": "#e599f7", | ||
| orange: "#f76707", | ||
| red: "#e03131", | ||
| violet: "#ae3ec9", | ||
| white: "#ffffff", | ||
| yellow: "#ffc078", | ||
| }; | ||
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.
I can see this making a lot of people that already set their relation color frustrated/confused. As long as we don't think a lot of users have set a new color, this should be fine. Otherwise we might want a simple hex converter.