MarkdownEditor polish

.changeset/metal-hornets-appear.md

@@ -0,0 +1,7 @@
+---
+"@primer/react": patch
+---
+
+Changes visual appearance of MarkdownEditor
+
+<!-- Changed components: MarkdownEditor -->

src/drafts/MarkdownEditor/Actions.tsx

@@ -7,6 +7,6 @@ Actions.displayName = 'MarkdownEditor.Actions'
 
 export const ActionButton = forwardRef<HTMLButtonElement, ButtonProps>((props, ref) => {
   const {disabled} = useContext(MarkdownEditorContext)
-  return <Button ref={ref} size="small" disabled={disabled} {...props} />
+  return <Button ref={ref} disabled={disabled} {...props} />
 })
 ActionButton.displayName = 'MarkdownEditor.ActionButton'

src/drafts/MarkdownEditor/Footer.tsx

@@ -1,38 +1,36 @@
 import React, {memo, forwardRef, useContext} from 'react'
-import {AlertIcon, ImageIcon, MarkdownIcon} from '@primer/octicons-react'
+import {PaperclipIcon} from '@primer/octicons-react'
 
-import {Spinner, LinkButton, Box, Text} from '../..'
+import {Spinner, Box, Text} from '../..'
 import {Button, ButtonProps} from '../../Button'
 import {MarkdownEditorContext} from './_MarkdownEditorContext'
 import {useSlots} from '../../hooks/useSlots'
 
 const uploadingNote = ([current, total]: [number, number]) =>
-  total === 1 ? `Uploading your file...` : `Uploading your files... (${current}/${total})`
+  total === 1 ? `Uploading your file…` : `Uploading your files… (${current}/${total})`
 
 export const CoreFooter = ({children}: {children: React.ReactNode}) => {
   const [slots, childrenWithoutSlots] = useSlots(children, {
     footerButtons: FooterButton,
   })
 
-  const {fileUploadProgress, errorMessage, previewMode} = useContext(MarkdownEditorContext)
+  const {fileUploadProgress, previewMode} = useContext(MarkdownEditorContext)
 
   return (
-    <Box sx={{pt: 2, display: 'flex', gap: 2, justifyContent: 'space-between', minHeight: '36px'}} as="footer">
+    <Box
+      sx={{pt: 2, display: 'flex', gap: 2, justifyContent: 'space-between', alignItems: 'center', minHeight: '36px'}}
+      as="footer"
+    >
       <Box sx={{display: 'flex', gap: 1, alignItems: 'center', fontSize: 0}}>
         {previewMode ? (
           <></>
         ) : fileUploadProgress ? (
-          <Text sx={{py: 1, px: 2}}>
+          <Text sx={{py: 1, px: 2, color: 'fg.muted'}}>
             <Spinner size="small" sx={{mr: 1, verticalAlign: 'text-bottom'}} /> {uploadingNote(fileUploadProgress)}
           </Text>
-        ) : errorMessage ? (
-          <ErrorMessage message={errorMessage} />
-        ) : (
-          <>
-            {slots.footerButtons && <Box sx={{display: 'flex', gap: 2}}>{slots.footerButtons}</Box>}
-            <DefaultFooterButtons />
-          </>
-        )}
+        ) : null}
+        {slots.footerButtons && <Box sx={{display: 'flex', gap: 2}}>{slots.footerButtons}</Box>}
+        <DefaultFooterButtons />
       </Box>
       {!fileUploadProgress && <Box sx={{display: 'flex', gap: 2}}>{childrenWithoutSlots}</Box>}
     </Box>
@@ -51,39 +49,19 @@ FooterButton.displayName = 'MarkdownEditor.FooterButton'
 const DefaultFooterButtons = memo(() => {
   const {uploadButtonProps, fileDraggedOver} = useContext(MarkdownEditorContext)
 
-  return (
-    <>
-      <MarkdownSupportedHint />
-
-      {uploadButtonProps && (
-        <>
-          <VisualSeparator />
-          <FileUploadButton fileDraggedOver={fileDraggedOver} {...uploadButtonProps} />
-        </>
-      )}
-    </>
-  )
+  return uploadButtonProps ? <FileUploadButton fileDraggedOver={fileDraggedOver} {...uploadButtonProps} /> : null
 })
 DefaultFooterButtons.displayName = 'MarkdownEditor.DefaultFooterButtons'
 
-const ErrorMessage = memo(({message}: {message: string}) => (
-  <Text sx={{py: 1, px: 2, color: 'danger.fg'}} aria-live="polite">
-    <Text sx={{mr: 1}}>
-      <AlertIcon size="small" />
-    </Text>{' '}
-    {message}
-  </Text>
-))
-
 const FileUploadButton = memo(({fileDraggedOver, ...props}: Partial<ButtonProps> & {fileDraggedOver: boolean}) => {
   const {condensed, disabled} = useContext(MarkdownEditorContext)
 
   return (
     <Button
       variant="invisible"
-      leadingVisual={ImageIcon}
+      leadingVisual={PaperclipIcon}
       size="small"
-      sx={{color: 'fg.default', fontWeight: fileDraggedOver ? 'bold' : 'normal', px: 2}}
+      sx={{color: 'fg.muted', fontWeight: 'normal', px: 2}}
       onMouseDown={(e: React.MouseEvent) => {
         // Prevent pulling focus from the textarea
         e.preventDefault()
@@ -95,29 +73,3 @@ const FileUploadButton = memo(({fileDraggedOver, ...props}: Partial<ButtonProps>
     </Button>
   )
 })
-
-const VisualSeparator = memo(() => (
-  <Box sx={{borderRightStyle: 'solid', borderRightWidth: 1, borderRightColor: 'border.muted', height: '100%'}} />
-))
-
-const MarkdownSupportedHint = memo(() => {
-  const {condensed} = useContext(MarkdownEditorContext)
-
-  return (
-    <LinkButton
-      leadingVisual={MarkdownIcon}
-      variant="invisible"
-      size="small"
-      sx={{color: 'inherit', fontWeight: 'normal', px: 2}}
-      href="https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax"
-      target="_blank"
-      // The markdown editor aria-description already describes it as Markdown editor, so it's
-      // redundant to say Markdown is supported again here. However for sighted users, they
-      // cannot see the aria-description so this is a useful hint for them. So the aria-label
-      // is different from the visible text content.
-      aria-label="Markdown documentation"
-    >
-      {!condensed && <Text aria-hidden>Markdown is supported</Text>}
-    </LinkButton>
-  )
-})

src/drafts/MarkdownEditor/MarkdownEditor.test.tsx

@@ -55,12 +55,10 @@ const render = async (ui: React.ReactElement) => {
 
   const queryForToolbarButton = (label: string) => within(getToolbar()).queryByRole('button', {name: label})
 
-  const getDefaultFooterButton = () => within(getFooter()).getByRole('link', {name: 'Markdown documentation'})
-
   const getActionButton = (label: string) => within(getFooter()).getByRole('button', {name: label})
 
   const getViewSwitch = () => {
-    const button = result.queryByRole('button', {name: 'Preview'}) || result.queryByRole('button', {name: 'Edit'})
+    const button = result.queryByRole('tab', {name: 'Preview'}) || result.queryByRole('tab', {name: 'Edit'})
     if (!button) throw new Error('View switch button not found')
     return button
   }
@@ -97,7 +95,6 @@ const render = async (ui: React.ReactElement) => {
     user,
     queryForUploadButton,
     getFooter,
-    getDefaultFooterButton,
     getViewSwitch,
     getPreview,
     queryForPreview,
@@ -298,13 +295,8 @@ describe('MarkdownEditor', () => {
   })
 
   describe('footer', () => {
-    it('renders default when not using custom footer', async () => {
-      const {getDefaultFooterButton} = await render(<UncontrolledEditor></UncontrolledEditor>)
-      expect(getDefaultFooterButton()).toBeInTheDocument()
-    })
-
     it('renders custom buttons', async () => {
-      const {getActionButton, getDefaultFooterButton} = await render(
+      const {getActionButton} = await render(
         <UncontrolledEditor>
           <MarkdownEditor.Footer>
             <MarkdownEditor.FooterButton>Footer A</MarkdownEditor.FooterButton>
@@ -315,12 +307,11 @@ describe('MarkdownEditor', () => {
         </UncontrolledEditor>,
       )
       expect(getActionButton('Footer A')).toBeInTheDocument()
-      expect(getDefaultFooterButton()).toBeInTheDocument()
       expect(getActionButton('Action A')).toBeInTheDocument()
     })
 
     it('disables buttons when the editor is disabled (unless explicitly overridden)', async () => {
-      const {getActionButton, getDefaultFooterButton} = await render(
+      const {getActionButton} = await render(
         <UncontrolledEditor disabled>
           <MarkdownEditor.Footer>
             <MarkdownEditor.FooterButton>Footer A</MarkdownEditor.FooterButton>
@@ -332,7 +323,6 @@ describe('MarkdownEditor', () => {
         </UncontrolledEditor>,
       )
       expect(getActionButton('Footer A')).toBeDisabled()
-      expect(getDefaultFooterButton()).not.toBeDisabled()
       expect(getActionButton('Action A')).toBeDisabled()
       expect(getActionButton('Action B')).not.toBeDisabled()
     })
@@ -710,7 +700,7 @@ describe('MarkdownEditor', () => {
 
       it('rejects disallows file types while accepting allowed ones', async () => {
         const onChange = jest.fn()
-        const {getInput, getFooter} = await render(
+        const {getInput, getEditorContainer} = await render(
           <UncontrolledEditor onUploadFile={mockUploadFile} onChange={onChange} acceptedFileTypes={['image/*']} />,
         )
         const input = getInput()
@@ -725,7 +715,7 @@ describe('MarkdownEditor', () => {
 
         await expectFilesToBeAdded(onChange, fileB)
 
-        expect(getFooter()).toHaveTextContent('File type not allowed: .app')
+        expect(getEditorContainer()).toHaveTextContent('File type not allowed: .app')
       })
 
       it('inserts "failed to upload" note on failure', async () => {