Skip to content

Commit 7cd4348

Browse files
sid597sid597
committed
address review
1 parent 0ef3c12 commit 7cd4348

3 files changed

Lines changed: 41 additions & 31 deletions

File tree

‎apps/roam/src/components/settings/DiscourseNodeCanvasSettings.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
5555
value={color}
5656
onChange={(e) => {
5757
setColor(e.target.value);
58-
setInputSetting({
58+
void setInputSetting({
5959
blockUid: uid,
6060
key: "color",
6161
value: e.target.value.replace("#", ""), // remove hash to not create roam link
@@ -68,7 +68,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
6868
icon={color ? "delete" : "info-sign"}
6969
onClick={() => {
7070
setColor("");
71-
setInputSetting({
71+
void setInputSetting({
7272
blockUid: uid,
7373
key: "color",
7474
value: "",
@@ -84,7 +84,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
8484
value={alias}
8585
onChange={(e) => {
8686
setAlias(e.target.value);
87-
setInputSetting({
87+
void setInputSetting({
8888
blockUid: uid,
8989
key: "alias",
9090
value: e.target.value,
@@ -101,7 +101,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
101101
onChange={(checked) => {
102102
setIsKeyImage(checked);
103103
if (checked && !keyImageOption) setKeyImageOption("first-image");
104-
setInputSetting({
104+
void setInputSetting({
105105
blockUid: uid,
106106
key: "key-image",
107107
value: checked ? "true" : "false",
@@ -115,7 +115,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
115115
onChange={(e) => {
116116
const value = (e.target as HTMLInputElement).value;
117117
setKeyImageOption(value);
118-
setInputSetting({
118+
void setInputSetting({
119119
blockUid: uid,
120120
key: "key-image-option",
121121
value,
@@ -140,7 +140,7 @@ const DiscourseNodeCanvasSettings = ({ nodeType, uid }: { nodeType: string; uid:
140140
value={queryBuilderAlias}
141141
onChange={(e) => {
142142
setQueryBuilderAlias(e.target.value);
143-
setInputSetting({
143+
void setInputSetting({
144144
blockUid: uid,
145145
key: "query-builder-alias",
146146
value: e.target.value,

‎apps/roam/src/components/settings/NodeConfig.tsx‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,11 @@ const NodeConfig = ({
148148
const shortcutUid = getUid("Shortcut");
149149
const tagUid = getUid("Tag");
150150
const templateUid = getUid("Template");
151-
const specificationUid = getUid("Specification");
152-
const indexUid = getUid("Index");
153151
const overlayUid = getUid("Overlay");
154-
const graphOverviewUid = getUid("Graph Overview");
155152
const canvasUid = getUid("Canvas");
153+
const graphOverviewUid = getUid("Graph Overview");
154+
const specificationUid = getUid("Specification");
155+
const indexUid = getUid("Index");
156156
const suggestiveRulesUid = getUid("Suggestive Rules");
157157
const attributeNode = getSubTree({
158158
parentUid: node.type,
@@ -167,11 +167,7 @@ const NodeConfig = ({
167167
value: formatValue,
168168
handleChange: handleFormatChange,
169169
handleBlur: handleFormatBlurFromHook,
170-
} = useDebouncedRoamUpdater(
171-
formatUid,
172-
node.format,
173-
!formatError,
174-
);
170+
} = useDebouncedRoamUpdater(formatUid, node.format, !formatError);
175171

176172
const validateTag = useCallback(
177173
(tag: string): string | undefined => {
@@ -193,9 +189,11 @@ const NodeConfig = ({
193189

194190
const validateFormat = useCallback(
195191
({
192+
tag,
196193
format,
197194
isSpecificationEnabled,
198195
}: {
196+
tag: string;
199197
format: string;
200198
isSpecificationEnabled?: boolean;
201199
}) => {
@@ -208,7 +206,7 @@ const NodeConfig = ({
208206
setFormatError("Error: you must set either a format or specification");
209207
return;
210208
}
211-
const cleanTag = getCleanTagText(tagValue);
209+
const cleanTag = getCleanTagText(tag);
212210

213211
if (!cleanTag) {
214212
setFormatError("");
@@ -229,23 +227,23 @@ const NodeConfig = ({
229227

230228
if (hasConflict) {
231229
setFormatError(
232-
`The format references the node's tag "${tagValue}". Please use a different format or tag.`,
230+
`The format references the node's tag "${tag}". Please use a different format or tag.`,
233231
);
234232
} else {
235233
setFormatError("");
236234
}
237235
},
238-
[specificationUid, tagValue],
236+
[specificationUid],
239237
);
240238

241239
useEffect(() => {
242-
validateFormat({ format: formatValue });
240+
validateFormat({ tag: tagValue, format: formatValue });
243241
}, [tagValue, formatValue, validateFormat]);
244242

245243
const handleFormatBlur = useCallback(() => {
246244
handleFormatBlurFromHook();
247-
validateFormat({ format: formatValue });
248-
}, [handleFormatBlurFromHook, formatValue, validateFormat]);
245+
validateFormat({ tag: tagValue, format: formatValue });
246+
}, [handleFormatBlurFromHook, tagValue, formatValue, validateFormat]);
249247

250248
return (
251249
<>
@@ -286,7 +284,7 @@ const NodeConfig = ({
286284
settingKeys={["tag"]}
287285
defaultValue={node.tag}
288286
placeholder={generateTagPlaceholder(node)}
289-
validate={validateTag}
287+
getValidationError={validateTag}
290288
onChange={setTagValue}
291289
order={2}
292290
parentUid={node.type}
@@ -333,6 +331,7 @@ const NodeConfig = ({
333331
parentUid={specificationUid}
334332
parentSetEnabled={(isSpecificationEnabled) => {
335333
validateFormat({
334+
tag: tagValue,
336335
format: formatValue,
337336
isSpecificationEnabled,
338337
});
@@ -382,7 +381,10 @@ const NodeConfig = ({
382381
title="Canvas"
383382
panel={
384383
<div className="flex flex-col gap-4 p-1">
385-
<DiscourseNodeCanvasSettings nodeType={node.type} uid={canvasUid} />
384+
<DiscourseNodeCanvasSettings
385+
nodeType={node.type}
386+
uid={canvasUid}
387+
/>
386388
<DiscourseNodeFlagPanel
387389
nodeType={node.type}
388390
title="Graph Overview"
@@ -402,7 +404,10 @@ const NodeConfig = ({
402404
title="Suggestive mode"
403405
panel={
404406
<div className="flex flex-col gap-4 p-1">
405-
<DiscourseNodeSuggestiveRules node={node} parentUid={suggestiveRulesUid} />
407+
<DiscourseNodeSuggestiveRules
408+
node={node}
409+
parentUid={suggestiveRulesUid}
410+
/>
406411
</div>
407412
}
408413
/>

‎apps/roam/src/components/settings/components/BlockPropSettingPanels.tsx‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ type FlagSetter = (keys: string[], value: boolean) => void;
3535
type NumberSetter = (keys: string[], value: number) => void;
3636

3737
type MultiTextSetter = (keys: string[], value: string[]) => void;
38+
type ValidationError = string;
39+
type Validator<T> = (value: T) => ValidationError | undefined;
3840

3941
type BaseTextPanelProps = {
4042
title: string;
@@ -43,7 +45,7 @@ type BaseTextPanelProps = {
4345
setter: TextSetter;
4446
initialValue?: string;
4547
placeholder?: string;
46-
validate?: (value: string) => string | undefined;
48+
getValidationError?: Validator<string>;
4749
onChange?: (value: string) => void;
4850
} & RoamBlockSyncProps;
4951

@@ -88,7 +90,7 @@ type BaseMultiTextPanelProps = {
8890
onChange?: (values: string[]) => void;
8991
} & RoamBlockSyncProps;
9092

91-
const DEBOUNCE_MS = 500;
93+
const DEBOUNCE_MS = 250;
9294

9395
const BaseTextPanel = ({
9496
title,
@@ -97,16 +99,14 @@ const BaseTextPanel = ({
9799
setter,
98100
initialValue,
99101
placeholder,
100-
validate,
102+
getValidationError,
101103
onChange,
102104
parentUid,
103105
uid,
104106
order,
105107
}: BaseTextPanelProps) => {
106108
const [value, setValue] = useState(() => initialValue ?? "");
107-
const [error, setError] = useState<string | undefined>(() =>
108-
validate?.(value),
109-
);
109+
const error = getValidationError?.(value);
110110
const debounceRef = useRef(0);
111111
const hasBlockSync = parentUid !== undefined && order !== undefined;
112112
const { onChange: rawSyncToBlock } = useSingleChildValue({
@@ -129,7 +129,7 @@ const BaseTextPanel = ({
129129
setValue(newValue);
130130
onChange?.(newValue);
131131

132-
if (validate?.(newValue)) return;
132+
if (getValidationError?.(newValue)) return;
133133

134134
window.clearTimeout(debounceRef.current);
135135
debounceRef.current = window.setTimeout(() => {
@@ -575,7 +575,12 @@ type DiscourseNodeBaseProps = {
575575
export const DiscourseNodeTextPanel = ({
576576
nodeType,
577577
...props
578-
}: DiscourseNodeBaseProps & RoamBlockSyncProps & { defaultValue?: string; placeholder?: string; validate?: (value: string) => string | undefined; onChange?: (value: string) => void }) => (
578+
}: DiscourseNodeBaseProps & RoamBlockSyncProps & {
579+
defaultValue?: string;
580+
placeholder?: string;
581+
getValidationError?: Validator<string>;
582+
onChange?: (value: string) => void;
583+
}) => (
579584
<BaseTextPanel
580585
{...props}
581586
setter={createDiscourseNodeSetter(nodeType)}

0 commit comments

Comments
 (0)