Skip to content

Commit 5c286ae

Browse files
authored
fix: variables not clickable in dialog in if-then branch (#784)
## Problem 1. SuggestionsPopper in Menubar dialog closes pre-emptively on click. This happens when RichTextEditors are rendered within a If-Then branch modal. 2. SuggestionsPopper in RichTextEditor does not reappear when the Menubar dialog closes. 3. Hyperlinks clicked in RichTextEditor are not substituted with test values resulting in links such as https://example.com/{{step.1.123123.answer}} ## Solution 1. To prevent z-index issues, migrate the dialog over to Chakra library to ensure consistency. 2. Remove onFocus handler with improperly keeps the SuggestionsPopper open 3. Prevent links from being clicked directly. Instead allow users to test their links in the Set Link Dialog <img width="687" alt="image" src="https://github.com/user-attachments/assets/0f291a45-dbc5-4fcb-a446-000c49942f53">
1 parent 5aa329b commit 5c286ae

File tree

6 files changed

+173
-120
lines changed

6 files changed

+173
-120
lines changed

packages/frontend/src/components/RichTextEditor/MenuBar.scss

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,3 @@
3232
background-color: #303030;
3333
}
3434
}
35-
36-
.menubar-dialog {
37-
.MuiDialog-container {
38-
// so that the variable drop down will have enough space even in small screen
39-
// to show "See all"
40-
align-items: flex-start;
41-
}
42-
.MuiDialogContentText-root .MuiButton-root {
43-
margin-top: 16px;
44-
}
45-
}

packages/frontend/src/components/RichTextEditor/MenuBar.tsx

Lines changed: 142 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import './MenuBar.scss'
22

3-
import { useCallback, useState } from 'react'
3+
import { useCallback, useMemo, useRef, useState } from 'react'
44
import { LuHeading1, LuHeading2, LuHeading3, LuHeading4 } from 'react-icons/lu'
55
import {
66
RiArrowGoBackLine,
@@ -20,18 +20,25 @@ import {
2020
RiTableLine,
2121
RiUnderline,
2222
} from 'react-icons/ri'
23-
import { LoadingButton } from '@mui/lab'
2423
import {
25-
Dialog,
26-
DialogContent,
27-
DialogContentText,
28-
DialogTitle,
29-
} from '@mui/material'
24+
AlertDialog,
25+
AlertDialogBody,
26+
AlertDialogCloseButton,
27+
AlertDialogContent,
28+
AlertDialogFooter,
29+
AlertDialogHeader,
30+
AlertDialogOverlay,
31+
Box,
32+
useDisclosure,
33+
} from '@chakra-ui/react'
34+
import { Button, Link } from '@opengovsg/design-system-react'
3035
import { Editor } from '@tiptap/react'
3136
import { parse } from 'node-html-parser'
3237

3338
import Form from '@/components/Form'
39+
import { makeExternalLink } from '@/helpers/urls'
3440

41+
import { simpleSubstitute, type VariableInfoMap } from './utils'
3542
import { BareEditor } from '.'
3643

3744
enum MenuLabels {
@@ -221,76 +228,98 @@ const menuButtons = [
221228
},
222229
]
223230

231+
const dialogPlaceholders: Partial<Record<MenuLabels, string>> = {
232+
[MenuLabels.LinkSet]: 'Enter a full URL with http prefix',
233+
[MenuLabels.ImageAdd]:
234+
'Enter direct image link (e.g. https://file.go.gov.sg/clipplumber.png)',
235+
}
236+
224237
interface MenuBarProps {
225238
editor: Editor | null
239+
variableMap: VariableInfoMap
226240
}
227-
export const MenuBar = ({ editor }: MenuBarProps) => {
228-
const [showValueDialog, setShowValueDialog] = useState(false)
241+
242+
export const MenuBar = ({ editor, variableMap }: MenuBarProps) => {
243+
const {
244+
isOpen: isDialogOpen,
245+
onClose,
246+
onOpen: onDialogOpen,
247+
} = useDisclosure()
248+
const cancelRef = useRef(null)
229249
const [dialogValue, setDialogValue] = useState('')
230250
const [dialogLabel, setDialogLabel] = useState<MenuLabels | null>(null)
231-
const onClickOverrides: { [key: string]: () => void } = {
232-
[MenuLabels.LinkSet]: useCallback(() => {
233-
if (!editor) {
234-
return
235-
}
236-
const previousUrl = editor.getAttributes('link').href
237-
setDialogLabel(MenuLabels.LinkSet)
238-
setDialogValue(previousUrl)
239-
setShowValueDialog(true)
240-
}, [editor]),
241-
[MenuLabels.ImageAdd]: useCallback(() => {
242-
if (!editor) {
243-
return
244-
}
245-
setDialogLabel(MenuLabels.ImageAdd)
246-
setDialogValue('')
247-
setShowValueDialog(true)
248-
}, [editor]),
249-
}
250-
const dialogOnSubmits: { [key: string]: () => void } = {
251-
[MenuLabels.LinkSet]: useCallback(() => {
252-
if (!editor) {
253-
return
254-
}
255-
const url = dialogValue
256-
// cancelled
257-
if (url === null) {
258-
return
259-
}
260251

261-
// empty
262-
if (url === '') {
263-
editor.chain().focus().extendMarkRange('link').unsetLink().run()
252+
const onDialogClose = useCallback(() => {
253+
setDialogLabel(null)
254+
setDialogValue('')
255+
onClose()
256+
}, [onClose])
264257

265-
return
266-
}
258+
const onClickOverrides: Partial<Record<MenuLabels, () => void>> = useMemo(
259+
() => ({
260+
[MenuLabels.LinkSet]: () => {
261+
if (!editor) {
262+
return
263+
}
264+
const previousUrl = editor.getAttributes('link').href
265+
setDialogLabel(MenuLabels.LinkSet)
266+
setDialogValue(previousUrl)
267+
onDialogOpen()
268+
},
269+
[MenuLabels.ImageAdd]: () => {
270+
if (!editor) {
271+
return
272+
}
273+
setDialogLabel(MenuLabels.ImageAdd)
274+
setDialogValue('')
275+
onDialogOpen()
276+
},
277+
}),
278+
[editor, onDialogOpen],
279+
)
280+
281+
const dialogOnSubmits: Partial<Record<MenuLabels, () => void>> = useMemo(
282+
() => ({
283+
[MenuLabels.LinkSet]: () => {
284+
if (!editor) {
285+
return
286+
}
287+
const url = dialogValue
288+
// cancelled
289+
if (url === null) {
290+
return
291+
}
292+
293+
// empty
294+
if (url === '') {
295+
editor.chain().focus().extendMarkRange('link').unsetLink().run()
296+
return
297+
}
298+
299+
// update link
300+
editor
301+
.chain()
302+
.focus()
303+
.extendMarkRange('link')
304+
.setLink({ href: url })
305+
.run()
306+
onDialogClose()
307+
},
308+
[MenuLabels.ImageAdd]: () => {
309+
if (!editor) {
310+
return
311+
}
312+
const url = dialogValue
313+
if (url === null) {
314+
return
315+
}
316+
editor.chain().focus().setImage({ src: url }).run()
317+
onDialogClose()
318+
},
319+
}),
320+
[editor, dialogValue, onDialogClose],
321+
)
267322

268-
// update link
269-
editor
270-
.chain()
271-
.focus()
272-
.extendMarkRange('link')
273-
.setLink({ href: url })
274-
.run()
275-
setShowValueDialog(false)
276-
}, [editor, dialogValue, setShowValueDialog]),
277-
[MenuLabels.ImageAdd]: useCallback(() => {
278-
if (!editor) {
279-
return
280-
}
281-
const url = dialogValue
282-
if (url === null) {
283-
return
284-
}
285-
editor.chain().focus().setImage({ src: url }).run()
286-
setShowValueDialog(false)
287-
}, [editor, dialogValue]),
288-
}
289-
const dialogPlaceholders: { [key: string]: string } = {
290-
[MenuLabels.LinkSet]: 'Enter a full URL with http prefix',
291-
[MenuLabels.ImageAdd]:
292-
'Enter direct image link (e.g. https://file.go.gov.sg/clipplumber.png)',
293-
}
294323
if (!editor) {
295324
return null
296325
}
@@ -316,8 +345,9 @@ export const MenuBar = ({ editor }: MenuBarProps) => {
316345
}}
317346
className={`menu-item${isActive?.(editor) ? ' is-active' : ''}`}
318347
onClick={() => {
319-
if (onClickOverrides && onClickOverrides[label]) {
320-
onClickOverrides[label]()
348+
const clickOverride = onClickOverrides[label]
349+
if (clickOverride) {
350+
clickOverride()
321351
return
322352
}
323353
onClick(editor)
@@ -328,25 +358,20 @@ export const MenuBar = ({ editor }: MenuBarProps) => {
328358
)
329359
})}
330360
</div>
331-
<Dialog
332-
open={showValueDialog}
333-
className="menubar-dialog"
334-
onClose={() => setShowValueDialog(false)}
335-
sx={{ alignItems: 'flex-start', overflow: 'scroll' }}
336-
PaperProps={{
337-
// so that the variable selector float can overlay
338-
sx: { display: 'inline-table' },
339-
}}
340-
>
341-
<DialogTitle>{dialogLabel}</DialogTitle>
342-
<DialogContent>
343-
<DialogContentText
344-
tabIndex={-1}
345-
component="div"
346-
className="menubar-dialog-content"
347-
>
348-
{dialogLabel && (
349-
<Form onSubmit={() => dialogOnSubmits[dialogLabel]()}>
361+
{isDialogOpen && dialogLabel && (
362+
<AlertDialog
363+
leastDestructiveRef={cancelRef}
364+
motionPreset="none"
365+
returnFocusOnClose={true}
366+
onClose={onDialogClose}
367+
isOpen={true}
368+
>
369+
<AlertDialogOverlay />
370+
<Form onSubmit={() => dialogOnSubmits[dialogLabel]?.()}>
371+
<AlertDialogContent>
372+
<AlertDialogHeader>{dialogLabel}</AlertDialogHeader>
373+
<AlertDialogCloseButton />
374+
<AlertDialogBody ref={cancelRef}>
350375
<BareEditor
351376
// val is in HTML, need to parse back to plain text
352377
onChange={(val) => setDialogValue(parse(val).textContent)}
@@ -355,19 +380,33 @@ export const MenuBar = ({ editor }: MenuBarProps) => {
355380
variablesEnabled
356381
placeholder={dialogPlaceholders[dialogLabel]}
357382
/>
358-
<LoadingButton
359-
type="submit"
360-
variant="contained"
361-
color="primary"
362-
sx={{ boxShadow: 2 }}
363-
>
364-
Submit
365-
</LoadingButton>
366-
</Form>
367-
)}
368-
</DialogContentText>
369-
</DialogContent>
370-
</Dialog>
383+
<Box p={2}>
384+
<Link
385+
isExternal
386+
referrerPolicy="no-referrer"
387+
isDisabled={!dialogValue}
388+
target="_blank"
389+
href={
390+
dialogValue
391+
? makeExternalLink(
392+
simpleSubstitute(dialogValue, variableMap),
393+
)
394+
: undefined
395+
}
396+
>
397+
Open link
398+
</Link>
399+
</Box>
400+
</AlertDialogBody>
401+
<AlertDialogFooter>
402+
<Button colorScheme="primary" type="submit">
403+
Done
404+
</Button>
405+
</AlertDialogFooter>
406+
</AlertDialogContent>
407+
</Form>
408+
</AlertDialog>
409+
)}
371410
</>
372411
)
373412
}

packages/frontend/src/components/RichTextEditor/VariableBadge.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ const PLACEHOLDER_TEMPLATE_STEP_ID = '00000000-0000-0000-0000-000000000000'
77

88
export const VariableBadge = ({ node }: { node: Node }) => {
99
// this happens when there is no value mapped properly
10-
const isEmpty = node.attrs.value === ''
10+
const isEmpty = node.attrs.value === '' || node.attrs.value == null
11+
const value = String(node.attrs.value)
1112
const isTemplate = String(node.attrs.id).includes(
1213
PLACEHOLDER_TEMPLATE_STEP_ID,
1314
)
@@ -42,9 +43,9 @@ export const VariableBadge = ({ node }: { node: Node }) => {
4243
>
4344
{node.attrs.label}
4445
</Text>
45-
{node.attrs.value && (
46+
{!isEmpty && (
4647
<Text isTruncated maxW="50ch" color="base.content.medium">
47-
{node.attrs.value}
48+
{value}
4849
</Text>
4950
)}
5051
</Badge>

packages/frontend/src/components/RichTextEditor/index.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const RICH_TEXT_EXTENSIONS = [
5353
}),
5454
Link.configure({
5555
HTMLAttributes: { rel: null, target: '_blank' },
56+
openOnClick: false,
5657
}),
5758
Underline,
5859
Table.configure({
@@ -207,6 +208,7 @@ const Editor = ({
207208
matchWidth={true}
208209
isLazy
209210
lazyBehavior="unmount"
211+
onClose={closeSuggestions}
210212
isOpen={isSuggestionsOpen && variablesEnabled}
211213
>
212214
<div
@@ -218,14 +220,12 @@ const Editor = ({
218220
if (e.currentTarget.contains(e.relatedTarget)) {
219221
return
220222
}
221-
222223
closeSuggestions()
223224
}}
224-
onFocus={openSuggestions}
225225
>
226226
<PopoverTrigger>
227227
<Box>
228-
{isRich && <MenuBar editor={editor} />}
228+
{isRich && <MenuBar editor={editor} variableMap={varInfo} />}
229229
<EditorContent className="editor__content" editor={editor} />
230230
<PopoverContent
231231
w="100%"

packages/frontend/src/components/RichTextEditor/utils.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,20 @@ export type VariableInfoMap = Map<
1212

1313
const VARIABLE_REGEX =
1414
/({{step\.[\da-f]{8}-(?:[\da-f]{4}-){3}[\da-f]{12}(?:\.[\da-zA-Z-_ ]+)+}})/
15+
const GLOBAL_VARIABLE_REGEX = new RegExp(VARIABLE_REGEX, 'g')
16+
/**
17+
* Used to generate substituted string for hyperlink checking
18+
*/
19+
export function simpleSubstitute(
20+
original: string,
21+
varInfo: VariableInfoMap,
22+
): string {
23+
return original.replaceAll(GLOBAL_VARIABLE_REGEX, (match) => {
24+
const id = match.replace('{{', '').replace('}}', '')
25+
const varInfoForNode = varInfo.get(`{{${id}}}`)
26+
return varInfoForNode?.testRunValue || ''
27+
})
28+
}
1529

1630
export function genVariableInfoMap(
1731
stepsWithVariables: StepWithVariables[],

0 commit comments

Comments
 (0)