Skip to content
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

fix(link-editor): update behaviour so that we can save on link collection items #863

Merged
merged 28 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b200089
refactor: linkmodal
seaerchin Nov 6, 2024
b7a376a
chore: update props
seaerchin Nov 6, 2024
6fa524b
fix: discard previous value if does not start with https
seaerchin Nov 6, 2024
74ad870
fix: write to external store on change
seaerchin Nov 6, 2024
889e80b
fix: consistent behaviour for toggling between radios
seaerchin Nov 6, 2024
f566f6b
fix: standardise handling of radio tabs
seaerchin Nov 7, 2024
07c9b34
fix: overflow for links
seaerchin Nov 7, 2024
f871ce2
chore: remove unused files
seaerchin Nov 7, 2024
e005569
fix: remove extra imporst
seaerchin Nov 7, 2024
8e36f83
chore: remove usequery
seaerchin Nov 7, 2024
bf87ddd
chore: add stories
seaerchin Nov 7, 2024
77620f8
chore: add stories
seaerchin Nov 7, 2024
ccaa1ec
fix: remove errors
seaerchin Nov 8, 2024
159e04d
fix: copy
seaerchin Nov 8, 2024
e27b0dd
chore: copy
seaerchin Nov 8, 2024
e9015f2
fix: update copy
seaerchin Nov 11, 2024
099ec6c
Merge branch 'main' into fix/file-collections
seaerchin Nov 11, 2024
b5928d2
chore: add missing import
seaerchin Nov 11, 2024
5c61988
chore: handle optional case
seaerchin Nov 11, 2024
bd10a6c
chore: fix types
seaerchin Nov 11, 2024
9cced01
chore: fix copy
seaerchin Nov 11, 2024
c2e225c
chore: fix aria
seaerchin Nov 12, 2024
a5e2468
chore: use const
seaerchin Nov 12, 2024
4c2dfbe
fix: use pick rather than respecifying
seaerchin Nov 12, 2024
a1245ad
Merge branch 'main' into fix/file-collections
seaerchin Nov 15, 2024
b52aab7
chore: rename
seaerchin Nov 15, 2024
8a5114d
Merge branch 'main' of https://github.com/opengovsg/isomer into fix/f…
adriangohjw Nov 16, 2024
ddb6385
fix - remove additional story
adriangohjw Nov 16, 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
98 changes: 64 additions & 34 deletions apps/studio/src/components/PageEditor/LinkEditorModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useParams } from "next/navigation"
import type { IconType } from "react-icons"
import {
Box,
FormControl,
Expand All @@ -20,8 +20,12 @@ import {
import { isEmpty } from "lodash"
import { z } from "zod"

import type { LinkTypeMapping } from "~/features/editing-experience/components/LinkEditor/constants"
import type { LinkTypes } from "~/features/editing-experience/components/LinkEditor/constants"
import { LinkHrefEditor } from "~/features/editing-experience/components/LinkEditor"
import {
LinkEditorContextProvider,
useLinkEditor,
} from "~/features/editing-experience/components/LinkEditor/LinkEditorContext"
import { useQueryParse } from "~/hooks/useQueryParse"
import { useZodForm } from "~/lib/form"
import { getReferenceLink, getResourceIdFromReferenceLink } from "~/utils/link"
Expand All @@ -33,6 +37,10 @@ const editSiteSchema = z.object({
siteId: z.coerce.number(),
})

const linkSchema = z.object({
linkId: z.coerce.string().optional(),
})

interface PageLinkElementProps {
value: string
onChange: (value: string) => void
Expand Down Expand Up @@ -72,7 +80,7 @@ interface LinkEditorModalContentProps {
linkText?: string
linkHref?: string
onSave: (linkText: string, linkHref: string) => void
linkTypes: LinkTypeMapping
linkTypes: LinkEditorModalProps["linkTypes"]
}
seaerchin marked this conversation as resolved.
Show resolved Hide resolved

const LinkEditorModalContent = ({
Expand All @@ -84,7 +92,6 @@ const LinkEditorModalContent = ({
const {
handleSubmit,
setValue,
watch,
register,
formState: { errors },
} = useZodForm({
Expand All @@ -99,7 +106,7 @@ const LinkEditorModalContent = ({
linkText,
linkHref,
},
reValidateMode: "onBlur",
reValidateMode: "onChange",
})

const isEditingLink = !!linkText && !!linkHref
Expand All @@ -110,13 +117,12 @@ const LinkEditorModalContent = ({
({ linkText, linkHref }) => !!linkHref && onSave(linkText, linkHref),
)

const { siteId } = useQueryParse(editSiteSchema)
// TODO: This needs to be refactored urgently
// This is a hacky way of seeing what to render
// and ties the link editor to the url path.
// we should instead just pass the component directly rather than using slots

const { linkId } = useParams()
const { linkId } = useQueryParse(linkSchema)

return (
<ModalContent>
Expand Down Expand Up @@ -153,33 +159,20 @@ const LinkEditorModalContent = ({
)}

<Box>
<LinkHrefEditor
<LinkEditorContextProvider
linkTypes={linkTypes}
value={watch("linkHref") ?? ""}
onChange={(value) => setValue("linkHref", value)}
label="Link destination"
description="When this is clicked, open:"
isRequired
isInvalid={!!errors.linkHref}
pageLinkElement={
<PageLinkElement
value={watch("linkHref") ?? ""}
onChange={(value) => setValue("linkHref", value)}
/>
}
fileLinkElement={
<FileAttachment
siteId={siteId}
setHref={(linkHref) => {
setValue("linkHref", linkHref)
}}
/>
}
/>

{errors.linkHref?.message && (
<FormErrorMessage>{errors.linkHref.message}</FormErrorMessage>
)}
linkHref={linkHref ?? ""}
onChange={(href) => setValue("linkHref", href)}
error={errors.linkHref?.message}
>
<ModalLinkEditor
onChange={(value) => setValue("linkHref", value)}
/>

{errors.linkHref?.message && (
<FormErrorMessage>{errors.linkHref.message}</FormErrorMessage>
)}
</LinkEditorContextProvider>
</Box>
</ModalBody>

Expand All @@ -206,7 +199,13 @@ interface LinkEditorModalProps {
onSave: (linkText: string, linkHref: string) => void
isOpen: boolean
onClose: () => void
linkTypes: LinkTypeMapping
linkTypes: Record<
string,
{
icon: IconType
label: Capitalize<LinkTypes>
}
>
}
Comment on lines +200 to +206
Copy link
Contributor

Choose a reason for hiding this comment

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

use LinkTypeMapping instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i actually tried this and it didn't quite work. this is because when we say Record<LinkTypes, string>, actually what we mean is a 1:1 mapping (ie, every type in LinkType must have a corresponding string) which isn't quite what we want here. what we want is "some subset of LinkTypes to string" - i considered using Partial<Record<LinkTypes, string>>, but unfortunately that means that every type is now optional. hence, i just went iwth string for now

export const LinkEditorModal = ({
isOpen,
Expand All @@ -232,3 +231,34 @@ export const LinkEditorModal = ({
)}
</Modal>
)

const ModalLinkEditor = ({
onChange,
}: {
onChange: (value: string) => void
}) => {
const { error, curHref, setHref } = useLinkEditor()
const { siteId } = useQueryParse(editSiteSchema)
const handleChange = (value: string) => {
onChange(value)
setHref(value)
}

return (
<LinkHrefEditor
label="Link destination"
description="When this is clicked, open:"
isRequired
isInvalid={!!error}
pageLinkElement={
<PageLinkElement value={curHref} onChange={handleChange} />
}
fileLinkElement={
<FileAttachment
siteId={siteId}
setHref={(href) => handleChange(href ?? "")}
/>
}
/>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type { PropsWithChildren } from "react"
import { createContext, useContext, useState } from "react"

import type {
LinkTypeMapping,
LinkTypes,
} from "~/features/editing-experience/components/LinkEditor/constants"

export type LinkEditorContextReturn = ReturnType<typeof useLinkEditorContext>
const LinkEditorContext = createContext<LinkEditorContextReturn | undefined>(
undefined,
)

interface UseLinkEditorContextProps {
linkHref: string
linkTypes: Partial<LinkTypeMapping>
error?: string
onChange: (value: string) => void
}
const useLinkEditorContext = ({
linkHref,
linkTypes,
error,
onChange,
}: UseLinkEditorContextProps) => {
const [curType, setCurType] = useState<LinkTypes>("page")
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
const [curHref, setHref] = useState(linkHref)

return {
linkTypes,
curHref,
setHref: (value: string) => {
onChange(value)
setHref(value)
},
error,
curType,
setCurType,
}
}

export const LinkEditorContextProvider = ({
children,
...passthroughProps
}: PropsWithChildren<UseLinkEditorContextProps>) => {
const values = useLinkEditorContext(passthroughProps)
return (
<LinkEditorContext.Provider value={values}>
{children}
</LinkEditorContext.Provider>
)
}

export const useLinkEditor = () => {
const context = useContext(LinkEditorContext)
if (!context) {
throw new Error(
`useLinkEditor must be used within a LinkEditorContextProvider component`,
)
}
return context
}
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import type { UseRadioProps } from "@chakra-ui/react"
import type { PropsWithChildren } from "react"
import {
Box,
HStack,
Icon,
Text,
useRadio,
useRadioGroup,
} from "@chakra-ui/react"

import type { LinkTypes } from "./constants"
import { useLinkEditor } from "./LinkEditorContext"

const LinkTypeRadioCard = ({
children,
...rest
}: PropsWithChildren<UseRadioProps>) => {
const { getInputProps, getRadioProps } = useRadio(rest)

return (
<Box
as="label"
_first={{
"> div": {
borderLeftRadius: "base",
},
}}
_last={{
"> div": {
borderRightRadius: "base",
},
}}
>
<input {...getInputProps()} />

<Box
{...getRadioProps()}
cursor="pointer"
border="1px solid"
borderColor="base.divider.strong"
bgColor="utility.ui"
px="1rem"
py="0.5rem"
mx={0}
_checked={{
bgColor: "interaction.muted.main.active",
color: "interaction.main.default",
borderColor: "interaction.main.default",
}}
textTransform="none"
fontWeight={500}
lineHeight="1.25rem"
>
{children}
</Box>
</Box>
)
}

export const LinkEditorRadio = () => {
const { linkTypes, setCurType } = useLinkEditor()
const { getRootProps, getRadioProps } = useRadioGroup({
name: "link-type",
defaultValue: "page",
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
// NOTE: This is a safe cast because we map over the `linkTypes` below
// so each time we are using the `linkType`
onChange: (value) => setCurType(value as LinkTypes),
adriangohjw marked this conversation as resolved.
Show resolved Hide resolved
})

return (
<HStack {...getRootProps()} spacing={0}>
{Object.entries(linkTypes).map(([key, props]) => {
if (!props) return null
const { icon, label } = props
const radio = getRadioProps({ value: key })

return (
<LinkTypeRadioCard key={key} {...radio}>
<HStack spacing={2}>
<Icon as={icon} fontSize="1.25rem" />
<Text textStyle="subhead-2">{label}</Text>
</HStack>
</LinkTypeRadioCard>
)
})}
</HStack>
)
}
Loading
Loading