fix(site-settings/navbar/footer): fix dogfoodign findings (#1718)

seaerchin · web-flow · commit 65fb92ca07ae · 2025-11-18T17:10:23.000+08:00
* fix: make object control always full widht

* fix: update equality comparison

* chore: links always open in new tab

* chore: cpoy changes

* chore: add error message

* chore: prevent infinite renders

* chore: fix equality check for integations

* chore: remove package

* chore: update deafult colour

* fix: update asPath

* chore: fix conditoinal

* chore: fix lints

* chore: fix lint findings

* chore: fix formatting
diff --git a/apps/studio/package.json b/apps/studio/package.json
@@ -157,6 +157,7 @@
     "react-hook-form": "^7.52.1",
     "react-icons": "^5.2.0",
     "react-markdown": "^10.1.0",
+    "rehype-external-links": "^3.0.0",
     "sass": "^1.83.4",
     "superjson": "^2.2.1",
     "tiptap-text-direction": "^0.3.1",
diff --git a/apps/studio/src/components/MarkdownLabel.tsx b/apps/studio/src/components/MarkdownLabel.tsx
@@ -1,5 +1,6 @@
 import { Box, useToken } from "@chakra-ui/react"
 import Markdown from "react-markdown"
+import rehypeExternalLinks from "rehype-external-links"
 
 export const MarkdownLabel = ({ description }: { description?: string }) => {
   const [linkColor, linkHoverColor, linkActiveColor] = useToken("colors", [
@@ -25,7 +26,9 @@ export const MarkdownLabel = ({ description }: { description?: string }) => {
         },
       }}
     >
-      <Markdown>{description}</Markdown>
+      <Markdown rehypePlugins={[[rehypeExternalLinks, { target: "_blank" }]]}>
+        {description}
+      </Markdown>
     </Box>
   )
 }
diff --git a/apps/studio/src/features/editing-experience/components/form-builder/renderers/controls/JsonFormsColourPickerControl.tsx b/apps/studio/src/features/editing-experience/components/form-builder/renderers/controls/JsonFormsColourPickerControl.tsx
@@ -11,7 +11,11 @@ import {
 } from "@chakra-ui/react"
 import { rankWith, schemaMatches } from "@jsonforms/core"
 import { useJsonForms, withJsonFormsControlProps } from "@jsonforms/react"
-import { FormLabel, Input } from "@opengovsg/design-system-react"
+import {
+  FormErrorMessage,
+  FormLabel,
+  Input,
+} from "@opengovsg/design-system-react"
 import get from "lodash/get"
 import { isHexadecimal } from "validator"
 
@@ -23,6 +27,8 @@ export const jsonFormsColourPickerControlTester: RankedTester = rankWith(
   schemaMatches((schema) => schema.format === "color-picker"),
 )
 
+export const DEFAULT_CONTENT_INVERSE_COLOUR = "2164da"
+
 const THEME_PATHS = [
   "colors.brand.canvas.default",
   "colors.brand.canvas.alt",
@@ -65,7 +71,7 @@ const JsonFormsColourPickerControl = ({
               </InputLeftAddon>
               <Input
                 value={displayedColour?.replace("#", "")}
-                placeholder="FFFFFF"
+                placeholder={DEFAULT_CONTENT_INVERSE_COLOUR}
                 w="86px"
                 onChange={(e) => {
                   const rawString = e.target.value
@@ -86,7 +92,10 @@ const JsonFormsColourPickerControl = ({
                   const palette = getPalette(parsedHex)
 
                   THEME_PATHS.forEach((p) => {
-                    handleChange(p, palette[p] ?? "#FFFFFF")
+                    handleChange(
+                      p,
+                      palette[p] ?? `#${DEFAULT_CONTENT_INVERSE_COLOUR}`,
+                    )
                   })
                 }}
               />
@@ -96,22 +105,29 @@ const JsonFormsColourPickerControl = ({
               border="1px solid"
               borderColor="base.divider.strong"
               bgColor={
-                displayedColour ? `#${normalizeHex(displayedColour)}` : "white"
+                displayedColour
+                  ? `#${normalizeHex(displayedColour)}`
+                  : `#${DEFAULT_CONTENT_INVERSE_COLOUR}`
               }
               w="2rem"
               h="2rem"
             ></Box>
           </HStack>
+          {!data && (
+            <FormErrorMessage>
+              Enter a hex code to generate a colour palette.
+            </FormErrorMessage>
+          )}
         </FormControl>
         <Box alignSelf="flex-start">
           <FormLabel
             description={
-              "This palette makes your website accessibility compliant."
+              "We’ve generated this palette from your brand colour. It makes your website compliant with accessibility standards."
             }
             mb={0}
             isRequired
           >
-            Color palette
+            Generated colour palette
           </FormLabel>
           <Flex mt="0.75rem" h="3rem">
             {THEME_PATHS.map((p) => {
@@ -129,7 +145,10 @@ const JsonFormsColourPickerControl = ({
                   borderColor="base.divider.medium"
                   borderLeftRadius={isFirst ? "6px" : "auto"}
                   borderRightRadius={isLast ? "6px" : "auto"}
-                  bgColor={get(ctx.core?.data as string | undefined, p)}
+                  bgColor={
+                    (get(ctx.core?.data, p) as string | undefined) ??
+                    `#${DEFAULT_CONTENT_INVERSE_COLOUR}`
+                  }
                 ></Box>
               )
             })}
diff --git a/apps/studio/src/features/editing-experience/components/form-builder/renderers/controls/JsonFormsObjectControl.tsx b/apps/studio/src/features/editing-experience/components/form-builder/renderers/controls/JsonFormsObjectControl.tsx
@@ -68,7 +68,7 @@ export function JsonFormsObjectControl({
 
   if (!required) {
     return (
-      <HStack spacing="0.5rem" alignItems="flex-start">
+      <HStack spacing="0.5rem" alignItems="flex-start" w="full">
         <VStack w="full" gap="0.75rem" pt="0.5rem" alignItems="start">
           <HStack alignItems="space-between" w="full" spacing="1rem">
             <FormControl
diff --git a/apps/studio/src/pages/sites/[siteId]/settings/footer.tsx b/apps/studio/src/pages/sites/[siteId]/settings/footer.tsx
@@ -3,6 +3,7 @@ import { useEffect, useState } from "react"
 import { useRouter } from "next/router"
 import { useToast } from "@opengovsg/design-system-react"
 import { ResourceType } from "~prisma/generated/generatedEnums"
+import isEqual from "lodash/isEqual"
 
 import { PermissionsBoundary } from "~/components/AuthWrappers"
 import {
@@ -52,7 +53,7 @@ const FooterSettingsPage: NextPageWithLayout = () => {
     FooterSchemaType | undefined
   >(content)
   const isOpen = !!nextUrl
-  const isDirty = JSON.stringify(previewFooterState) !== JSON.stringify(content)
+  const isDirty = !isEqual(previewFooterState, content)
 
   const handleSaveFooter = (data?: FooterSchemaType) => {
     if (!data) return
diff --git a/apps/studio/src/pages/sites/[siteId]/settings/integrations.tsx b/apps/studio/src/pages/sites/[siteId]/settings/integrations.tsx
@@ -12,7 +12,7 @@ import {
 } from "@opengovsg/isomer-components"
 import { Value } from "@sinclair/typebox/value"
 import { ResourceType } from "~prisma/generated/generatedEnums"
-import { isEqual } from "lodash"
+import { isEqual, pickBy } from "lodash"
 import { BiWrench } from "react-icons/bi"
 
 import type { NextPageWithLayout } from "~/lib/types"
@@ -82,8 +82,11 @@ const IntegrationsSettingsPage: NextPageWithLayout = () => {
     useState<ComplexIntegrationsSettings>({ askgov, vica })
 
   const isDirty = !isEqual(
-    { siteGtmId, search, askgov, vica },
-    { ...simpleIntegrationSettings, ...complexIntegrationSettings },
+    pickBy({ siteGtmId, search, askgov, vica }, (val) => !!val),
+    pickBy(
+      { ...simpleIntegrationSettings, ...complexIntegrationSettings },
+      (val) => !!val,
+    ),
   )
 
   const updateSiteIntegrationsMutation =
@@ -167,13 +170,19 @@ const IntegrationsSettingsPage: NextPageWithLayout = () => {
               data={complexIntegrationSettings}
               handleChange={(data) => {
                 if (data.vica) {
-                  setComplexIntegrationSettings({
-                    ...data,
-                    vica: {
-                      ...data.vica,
-                      "app-name": agencyName ?? siteName,
-                    },
-                  })
+                  const newVicaState = {
+                    ...data.vica,
+                    "app-name": agencyName ?? siteName,
+                  }
+
+                  if (!isEqual(newVicaState, data.vica)) {
+                    setComplexIntegrationSettings({
+                      ...data,
+                      vica: newVicaState,
+                    })
+                  } else {
+                    setComplexIntegrationSettings(data)
+                  }
                 } else {
                   setComplexIntegrationSettings(data)
                 }
diff --git a/apps/studio/src/pages/sites/[siteId]/settings/navbar.tsx b/apps/studio/src/pages/sites/[siteId]/settings/navbar.tsx
@@ -3,7 +3,9 @@ import { useEffect, useState } from "react"
 import { useRouter } from "next/router"
 import { useToast } from "@opengovsg/design-system-react"
 import { ResourceType } from "~prisma/generated/generatedEnums"
+import isEqual from "lodash/isEqual"
 
+import type { NextPageWithLayout } from "~/lib/types"
 import { PermissionsBoundary } from "~/components/AuthWrappers"
 import {
   SettingsEditorGridItem,
@@ -22,7 +24,6 @@ import { NavbarEditor } from "~/features/settings/NavbarEditor"
 import { useNavigationEffect } from "~/hooks/useNavigationEffect"
 import { useNewSettingsPage } from "~/hooks/useNewSettingsPage"
 import { useQueryParse } from "~/hooks/useQueryParse"
-import { type NextPageWithLayout } from "~/lib/types"
 import { SiteSettingsLayout } from "~/templates/layouts/SiteSettingsLayout"
 import { trpc } from "~/utils/trpc"
 
@@ -60,7 +61,7 @@ const NavbarSettingsPage: NextPageWithLayout = () => {
     NavbarSchemaType | undefined
   >(content)
   const isOpen = !!nextUrl
-  const isDirty = JSON.stringify(previewNavbarState) !== JSON.stringify(content)
+  const isDirty = !isEqual(previewNavbarState, content)
 
   const handleSaveNavbar = (data: NavbarSchemaType | undefined) => {
     if (!data) return
diff --git a/apps/studio/src/pages/sites/[siteId]/settings/notification.tsx b/apps/studio/src/pages/sites/[siteId]/settings/notification.tsx
@@ -109,7 +109,7 @@ const NotificationSettingsPage: NextPageWithLayout = () => {
             isLoading={notificationMutation.isPending}
             isDisabled={!isDirty}
           />
-          <Box>
+          <Box w="100%">
             <Box mb="-0.5rem" />
             <FormBuilder<Notification>
               schema={NotificationSettingsSchema}
diff --git a/apps/studio/src/stories/Page/SettingsPage/Colour.stories.tsx b/apps/studio/src/stories/Page/SettingsPage/Colour.stories.tsx
@@ -3,6 +3,7 @@ import { userEvent, within } from "storybook/test"
 import { pageHandlers } from "tests/msw/handlers/page"
 import { sitesHandlers } from "tests/msw/handlers/sites"
 
+import { DEFAULT_CONTENT_INVERSE_COLOUR } from "~/features/editing-experience/components/form-builder/renderers/controls/JsonFormsColourPickerControl"
 import ColoursSettingsPage from "~/pages/sites/[siteId]/settings/colours"
 import { ADMIN_HANDLERS } from "~/stories/handlers"
 
@@ -28,7 +29,7 @@ const meta: Meta<typeof ColoursSettingsPage> = {
     },
     nextjs: {
       router: {
-        asPath: "/sites/1/settings/logo",
+        asPath: "/sites/1/settings/colours",
         query: {
           siteId: "1",
         },
@@ -61,7 +62,9 @@ export const DarkPalette: Story = {
   play: async ({ canvasElement }) => {
     const rootScreen = within(canvasElement.ownerDocument.body)
 
-    const colourInput = await rootScreen.findByPlaceholderText("FFFFFF")
+    const colourInput = await rootScreen.findByPlaceholderText(
+      DEFAULT_CONTENT_INVERSE_COLOUR,
+    )
 
     await userEvent.clear(colourInput)
     await userEvent.type(colourInput, "#000000")
@@ -72,18 +75,22 @@ export const LightPalette: Story = {
   play: async ({ canvasElement }) => {
     const rootScreen = within(canvasElement.ownerDocument.body)
 
-    const colourInput = await rootScreen.findByPlaceholderText("FFFFFF")
+    const colourInput = await rootScreen.findByPlaceholderText(
+      DEFAULT_CONTENT_INVERSE_COLOUR,
+    )
 
     await userEvent.clear(colourInput)
-    await userEvent.type(colourInput, "FFFFFF")
+    await userEvent.type(colourInput, DEFAULT_CONTENT_INVERSE_COLOUR)
   },
 }
 
 export const Empty: Story = {
   play: async ({ canvasElement }) => {
     const rootScreen = within(canvasElement.ownerDocument.body)
 
-    const colourInput = await rootScreen.findByPlaceholderText("FFFFFF")
+    const colourInput = await rootScreen.findByPlaceholderText(
+      DEFAULT_CONTENT_INVERSE_COLOUR,
+    )
 
     await userEvent.clear(colourInput)
   },
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/components/src/templates/next/components/internal/Filter/Filter.stories.tsx b/packages/components/src/templates/next/components/internal/Filter/Filter.stories.tsx
@@ -12,7 +12,6 @@ const meta: Meta<typeof Filter> = {
   component: Filter,
   render: ({ filters, appliedFilters: _appliedFilters }) => {
     const [appliedFilters, setAppliedFilters] =
-      // eslint-disable-next-line react-hooks/rules-of-hooks
       useState<AppliedFilter[]>(_appliedFilters)
     const updateAppliedFilters = (
       appliedFilters: AppliedFilter[],
diff --git a/packages/components/src/templates/next/components/native/Prose/utils.ts b/packages/components/src/templates/next/components/native/Prose/utils.ts
@@ -1,4 +1,4 @@
-import { ProseProps } from "~/interfaces"
+import type { ProseProps } from "~/interfaces"
 
 export const hasContent = (content: ProseProps["content"]) => {
   // NOTE: top level is always `prose`
diff --git a/packages/components/src/templates/next/layouts/Collection/utils/getTagsFromTagged.ts b/packages/components/src/templates/next/layouts/Collection/utils/getTagsFromTagged.ts
@@ -1,4 +1,4 @@
-import { ArticlePagePageProps, CollectionPagePageProps } from "~/types"
+import type { ArticlePagePageProps, CollectionPagePageProps } from "~/types"
 
 export const getTagsFromTagged = (
   tagged: NonNullable<ArticlePagePageProps["tagged"]>,
diff --git a/packages/components/src/types/theme.ts b/packages/components/src/types/theme.ts
@@ -11,7 +11,8 @@ export const SiteThemeSchema = Type.Object(
           backdrop: Type.String({ format: "hidden" }),
           inverse: Type.String({
             title: "Main brand colour",
-            description: "Pick the main colour that represents your brand.",
+            description:
+              "Enter a hex code of a colour that best represents your brand. If you have a design system or branding guide, this might be your primary colour.",
             format: "color-picker",
           }),
         }),
diff --git a/packages/components/src/utils/getIndexByPermalink.ts b/packages/components/src/utils/getIndexByPermalink.ts
@@ -1,4 +1,4 @@
-import { IsomerSitemap } from "~/types"
+import type { IsomerSitemap } from "~/types"
 import { getNodeFromSiteMap } from "./getNodeFromSiteMap"
 
 export const getIndexByPermalink = (

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`import { Box, useToken } from "@chakra-ui/react"`
`2`	`2`	`import Markdown from "react-markdown"`
	`3`	`+import rehypeExternalLinks from "rehype-external-links"`
`3`	`4`
`4`	`5`	`export const MarkdownLabel = ({ description }: { description?: string }) => {`
`5`	`6`	`const [linkColor, linkHoverColor, linkActiveColor] = useToken("colors", [`
`@@ -25,7 +26,9 @@ export const MarkdownLabel = ({ description }: { description?: string }) => {`
`25`	`26`	`},`
`26`	`27`	`}}`
`27`	`28`	`>`
`28`		`- <Markdown>{description}</Markdown>`
	`29`	`+ <Markdown rehypePlugins={[[rehypeExternalLinks, { target: "_blank" }]]}>`
	`30`	`+ {description}`
	`31`	`+ </Markdown>`
`29`	`32`	`</Box>`
`30`	`33`	`)`
`31`	`34`	`}`