From c4243826d1d1827d0cddc3da5a0ca61c04564c24 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 13 Nov 2023 15:17:37 +1000 Subject: [PATCH 1/2] Revert "Fix MarkdownEditor focus variable usage and border bottom missing (#3911)" This reverts commit ef8d5bdda3874385b5536c487f340fa5fc5b3986. --- src/drafts/MarkdownEditor/MarkdownEditor.tsx | 21 ++------------------ 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.tsx index 03bcd6017b0..947909c5970 100644 --- a/src/drafts/MarkdownEditor/MarkdownEditor.tsx +++ b/src/drafts/MarkdownEditor/MarkdownEditor.tsx @@ -403,8 +403,7 @@ const MarkdownEditor = forwardRef( '&: focus-within': view === 'edit' ? { - outline: '2px solid', - outlineColor: 'accent.emphasis', + outline: '2px solid var(--borderColor-accent-emphasis)', } : {}, ...sx, @@ -425,29 +424,13 @@ const MarkdownEditor = forwardRef( }} as="header" > - + - From 5ad5fcbd14091daba864c610d94f6762fd8c6a87 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 13 Nov 2023 15:17:46 +1000 Subject: [PATCH 2/2] Revert "MarkdownEditor polish (#3667)" This reverts commit 791c983846115438bbea54768b73272260736007. --- .changeset/metal-hornets-appear.md | 7 - src/drafts/MarkdownEditor/Actions.tsx | 2 +- src/drafts/MarkdownEditor/Footer.tsx | 78 +++++++-- .../MarkdownEditor/MarkdownEditor.test.tsx | 20 ++- src/drafts/MarkdownEditor/MarkdownEditor.tsx | 163 ++++++++---------- src/drafts/MarkdownEditor/Toolbar.tsx | 37 +--- src/drafts/MarkdownEditor/_ErrorMessage.tsx | 13 -- src/drafts/MarkdownEditor/_MarkdownInput.tsx | 9 +- src/drafts/MarkdownEditor/_ViewSwitch.tsx | 48 +++--- 9 files changed, 179 insertions(+), 198 deletions(-) delete mode 100644 .changeset/metal-hornets-appear.md delete mode 100644 src/drafts/MarkdownEditor/_ErrorMessage.tsx diff --git a/.changeset/metal-hornets-appear.md b/.changeset/metal-hornets-appear.md deleted file mode 100644 index f9ab197087d..00000000000 --- a/.changeset/metal-hornets-appear.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"@primer/react": patch ---- - -Changes visual appearance of MarkdownEditor - - diff --git a/src/drafts/MarkdownEditor/Actions.tsx b/src/drafts/MarkdownEditor/Actions.tsx index a38df16dbdd..25055e623b9 100644 --- a/src/drafts/MarkdownEditor/Actions.tsx +++ b/src/drafts/MarkdownEditor/Actions.tsx @@ -7,6 +7,6 @@ Actions.displayName = 'MarkdownEditor.Actions' export const ActionButton = forwardRef((props, ref) => { const {disabled} = useContext(MarkdownEditorContext) - return ) }) + +const VisualSeparator = memo(() => ( + +)) + +const MarkdownSupportedHint = memo(() => { + const {condensed} = useContext(MarkdownEditorContext) + + return ( + + {!condensed && Markdown is supported} + + ) +}) diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx index 3c1509e4027..daa3809cc2c 100644 --- a/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx +++ b/src/drafts/MarkdownEditor/MarkdownEditor.test.tsx @@ -55,10 +55,12 @@ 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('tab', {name: 'Preview'}) || result.queryByRole('tab', {name: 'Edit'}) + const button = result.queryByRole('button', {name: 'Preview'}) || result.queryByRole('button', {name: 'Edit'}) if (!button) throw new Error('View switch button not found') return button } @@ -95,6 +97,7 @@ const render = async (ui: React.ReactElement) => { user, queryForUploadButton, getFooter, + getDefaultFooterButton, getViewSwitch, getPreview, queryForPreview, @@ -295,8 +298,13 @@ describe('MarkdownEditor', () => { }) describe('footer', () => { + it('renders default when not using custom footer', async () => { + const {getDefaultFooterButton} = await render() + expect(getDefaultFooterButton()).toBeInTheDocument() + }) + it('renders custom buttons', async () => { - const {getActionButton} = await render( + const {getActionButton, getDefaultFooterButton} = await render( Footer A @@ -307,11 +315,12 @@ describe('MarkdownEditor', () => { , ) 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} = await render( + const {getActionButton, getDefaultFooterButton} = await render( Footer A @@ -323,6 +332,7 @@ describe('MarkdownEditor', () => { , ) expect(getActionButton('Footer A')).toBeDisabled() + expect(getDefaultFooterButton()).not.toBeDisabled() expect(getActionButton('Action A')).toBeDisabled() expect(getActionButton('Action B')).not.toBeDisabled() }) @@ -700,7 +710,7 @@ describe('MarkdownEditor', () => { it('rejects disallows file types while accepting allowed ones', async () => { const onChange = jest.fn() - const {getInput, getEditorContainer} = await render( + const {getInput, getFooter} = await render( , ) const input = getInput() @@ -715,7 +725,7 @@ describe('MarkdownEditor', () => { await expectFilesToBeAdded(onChange, fileB) - expect(getEditorContainer()).toHaveTextContent('File type not allowed: .app') + expect(getFooter()).toHaveTextContent('File type not allowed: .app') }) it('inserts "failed to upload" note on failure', async () => { diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.tsx index 947909c5970..1ba81af9550 100644 --- a/src/drafts/MarkdownEditor/MarkdownEditor.tsx +++ b/src/drafts/MarkdownEditor/MarkdownEditor.tsx @@ -27,7 +27,6 @@ import {Emoji} from './suggestions/_useEmojiSuggestions' import {Mentionable} from './suggestions/_useMentionSuggestions' import {Reference} from './suggestions/_useReferenceSuggestions' import {isModifierKey} from './utils' -import {ErrorMessage} from './_ErrorMessage' import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' export type MarkdownEditorProps = SxProp & { @@ -384,55 +383,34 @@ const MarkdownEditor = forwardRef( sx={{ display: 'flex', flexDirection: 'column', + width: '100%', + borderColor: 'border.default', + borderWidth: 1, + borderStyle: 'solid', + borderRadius: 2, + p: 2, + height: fullHeight ? '100%' : undefined, + minInlineSize: 'auto', + bg: 'canvas.default', + color: disabled ? 'fg.subtle' : 'fg.default', + ...sx, }} ref={containerRef} > - - - Markdown input: - {view === 'preview' ? ' preview mode selected.' : ' edit mode selected.'} - - - - - - + + Markdown input: + {view === 'preview' ? ' preview mode selected.' : ' edit mode selected.'} + + + + + {view === 'edit' && (slots.toolbar ?? ( @@ -442,54 +420,53 @@ const MarkdownEditor = forwardRef( ))} - - - {view === 'edit' && fileHandler?.errorMessage && } - - {view === 'preview' && ( - -

Rendered Markdown Preview

- -
- )}
+ + + + {view === 'preview' && ( + +

Rendered Markdown Preview

+ +
+ )} {slots.footer ?? ( {React.isValidElement(slots.actions) && slots.actions.props.children} )} diff --git a/src/drafts/MarkdownEditor/Toolbar.tsx b/src/drafts/MarkdownEditor/Toolbar.tsx index dccc1f35333..0b975733298 100644 --- a/src/drafts/MarkdownEditor/Toolbar.tsx +++ b/src/drafts/MarkdownEditor/Toolbar.tsx @@ -22,36 +22,23 @@ import {MarkdownEditorContext} from './_MarkdownEditorContext' import {SavedRepliesButton} from './_SavedReplies' export const ToolbarButton = forwardRef((props, ref) => { - const {disabled, condensed} = useContext(MarkdownEditorContext) + const {disabled} = useContext(MarkdownEditorContext) return ( e.preventDefault()} {...props} - sx={{color: 'fg.muted', ...props.sx}} + sx={{color: 'fg.default', ...props.sx}} /> ) }) ToolbarButton.displayName = 'MarkdownEditor.ToolbarButton' -const Divider = () => { - return ( - - ) -} - export const DefaultToolbarButtons = memo(() => { const {condensed, formattingToolsRef} = useContext(MarkdownEditorContext) @@ -78,7 +65,6 @@ export const DefaultToolbarButtons = memo(() => { /> - formattingToolsRef.current?.quote()} icon={QuoteIcon} @@ -96,7 +82,6 @@ export const DefaultToolbarButtons = memo(() => { /> - formattingToolsRef.current?.unorderedList()} icon={ListUnorderedIcon} @@ -115,7 +100,6 @@ export const DefaultToolbarButtons = memo(() => { {!condensed && ( - formattingToolsRef.current?.mention()} icon={MentionIcon} @@ -135,6 +119,8 @@ export const DefaultToolbarButtons = memo(() => { DefaultToolbarButtons.displayName = 'MarkdownEditor.DefaultToolbarButtons' export const CoreToolbar = ({children}: {children?: React.ReactNode}) => { + const {condensed} = useContext(MarkdownEditorContext) + const containerRef = useRef(null) useFocusZone({ @@ -149,18 +135,7 @@ export const CoreToolbar = ({children}: {children?: React.ReactNode}) => { ref={containerRef} aria-label="Formatting tools" role="toolbar" - sx={{ - display: 'flex', - flexWrap: 'wrap', - justifyContent: 'flex-end', - gap: 0, - alignItems: 'center', - flexGrow: 1, - borderBottom: '1px solid', - borderBottomColor: 'border.muted', - pl: 2, - pr: 1, - }} + sx={{display: 'flex', flexWrap: 'wrap', justifyContent: 'flex-end', gap: condensed ? 0 : 3}} > {children} diff --git a/src/drafts/MarkdownEditor/_ErrorMessage.tsx b/src/drafts/MarkdownEditor/_ErrorMessage.tsx deleted file mode 100644 index 4e6c9675fe4..00000000000 --- a/src/drafts/MarkdownEditor/_ErrorMessage.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import React, {memo} from 'react' -import Flash from '../../Flash' - -export const ErrorMessage = memo(({message}: {message: string}) => ( - - {message} - -)) diff --git a/src/drafts/MarkdownEditor/_MarkdownInput.tsx b/src/drafts/MarkdownEditor/_MarkdownInput.tsx index e22a0858590..da6c02d5103 100644 --- a/src/drafts/MarkdownEditor/_MarkdownInput.tsx +++ b/src/drafts/MarkdownEditor/_MarkdownInput.tsx @@ -152,21 +152,18 @@ export const MarkdownInput = forwardRef aria-label="Markdown value" onChange={onChange} sx={{ + width: '100%', borderStyle: 'none', boxShadow: 'none', height: fullHeight ? '100%' : undefined, outline: theme => { - return isDraggedOver ? `dashed 2px ${theme.colors.border.muted}` : undefined + return isDraggedOver ? `solid 2px ${theme.colors.accent.fg}` : undefined }, - outlineOffset: '-4px', display: visible ? undefined : 'none', - '&: focus-within': { - boxShadow: 'none', - }, '& textarea': { lineHeight: 1.2, resize: fullHeight ? 'none' : 'vertical', - p: 3, + p: 2, fontFamily: monospace ? 'mono' : 'normal', ...heightStyles, }, diff --git a/src/drafts/MarkdownEditor/_ViewSwitch.tsx b/src/drafts/MarkdownEditor/_ViewSwitch.tsx index f1fb08482af..f510e3e7cc1 100644 --- a/src/drafts/MarkdownEditor/_ViewSwitch.tsx +++ b/src/drafts/MarkdownEditor/_ViewSwitch.tsx @@ -1,7 +1,9 @@ -import React from 'react' +import React, {useContext} from 'react' +import {EyeIcon, PencilIcon} from '@primer/octicons-react' import Box from '../../Box' -import TabNav from '../../TabNav' +import {Button, IconButton} from '../../Button' +import {MarkdownEditorContext} from './_MarkdownEditorContext' export type MarkdownViewMode = 'preview' | 'edit' @@ -16,47 +18,39 @@ type ViewSwitchProps = { // no point in memoizing this component because onLoadPreview depends on value, so it would still re-render on every change export const ViewSwitch = ({selectedView, onViewSelect, onLoadPreview, disabled}: ViewSwitchProps) => { // don't get disabled from context - the switch is not disabled when the editor is disabled + const {condensed} = useContext(MarkdownEditorContext) - const sharedProps = + const {label, icon, ...sharedProps} = selectedView === 'preview' ? { + variant: 'invisible' as const, + sx: {color: 'fg.default', px: 2}, onClick: () => onViewSelect?.('edit'), + icon: PencilIcon, + label: 'Edit', } : { + variant: 'invisible' as const, + sx: {color: 'fg.default', px: 2}, onClick: () => { onLoadPreview() onViewSelect?.('preview') }, onMouseOver: () => onLoadPreview(), onFocus: () => onLoadPreview(), + icon: EyeIcon, + label: 'Preview', } return ( - - - Write - - - Preview - - + {condensed ? ( + + ) : ( + + )} ) }