From 4320eb12140b595c7e9229681dd37f42e22cd5be Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 13 Sep 2023 17:43:20 +0200 Subject: [PATCH 01/44] Export ActionMenu/index for backward compatibility with #3713 (#3740) * add ActionMenu/index.ts to rollup entrypoint * add PR link to commment --- rollup.config.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rollup.config.js b/rollup.config.js index da1fa285478..2689076a07d 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -35,6 +35,9 @@ const input = new Set([ // "./lib-esm/utils/*" 'src/utils/*', + + // for backward compatbility, see https://github.com/primer/react/pull/3740 + 'src/ActionMenu/index.ts', ], { cwd: __dirname, From a3f14f7b42ba9928a401763157cb76830a990af8 Mon Sep 17 00:00:00 2001 From: Amanda Brown Date: Wed, 13 Sep 2023 19:16:47 -0400 Subject: [PATCH 02/44] Added MarkdownEditor story to testing (#3633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * added/shuffled MarkdownEditor stories * Added MarkdownEditor to storybook test file * refactor: fix lint issues * remove unnecessary imports * Update src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx Co-authored-by: Armağan * Update src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx Co-authored-by: Armağan * Update src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx Co-authored-by: Armağan * fix: switched out fragments to Box elements * fix: added decorator to MarkdownEditor Features story * Update src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx Co-authored-by: Armağan * Update src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx Co-authored-by: Armağan --------- Co-authored-by: Armağan --- src/__tests__/storybook.test.tsx | 1 + .../MarkdownEditor.features.stories.tsx | 424 ++++++++++++++++++ .../MarkdownEditor/MarkdownEditor.stories.tsx | 219 +-------- 3 files changed, 429 insertions(+), 215 deletions(-) create mode 100644 src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx diff --git a/src/__tests__/storybook.test.tsx b/src/__tests__/storybook.test.tsx index f9e25b9783b..eb674d09f9b 100644 --- a/src/__tests__/storybook.test.tsx +++ b/src/__tests__/storybook.test.tsx @@ -28,6 +28,7 @@ const allowlist = [ 'IconButton', 'FilteredActionList', 'Link', + 'MarkdownEditor', 'Pagehead', 'Pagination', 'ProgressBar', diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx new file mode 100644 index 00000000000..4f427de25cb --- /dev/null +++ b/src/drafts/MarkdownEditor/MarkdownEditor.features.stories.tsx @@ -0,0 +1,424 @@ +import {DiffIcon, PlusIcon} from '@primer/octicons-react' +import React, {Meta} from '@storybook/react' +import {useRef, useState} from 'react' +import Box from '../../Box' +import MarkdownEditor, {Emoji, Mentionable, Reference, SavedReply} from '.' + +const meta: Meta = { + title: 'Drafts/Components/MarkdownEditor/Features', + parameters: { + controls: { + include: [ + 'Disabled', + 'Full Height', + 'Monospace Font', + 'Minimum Height (Lines)', + 'Maximum Height (Lines)', + 'Hide Label', + 'Required', + 'Enable File Uploads', + 'Enable Saved Replies', + 'Enable Plain-Text URL Pasting', + ], + }, + }, + component: MarkdownEditor, + args: { + disabled: false, + fullHeight: false, + monospace: false, + pasteUrlsAsPlainText: false, + minHeightLines: 5, + maxHeightLines: 35, + hideLabel: false, + required: false, + fileUploadsEnabled: true, + savedRepliesEnabled: true, + }, + argTypes: { + disabled: { + name: 'Disabled', + control: { + type: 'boolean', + }, + }, + fullHeight: { + name: 'Full Height', + control: { + type: 'boolean', + }, + }, + monospace: { + name: 'Monospace Font', + control: { + type: 'boolean', + }, + }, + pasteUrlsAsPlainText: { + name: 'Enable Plain-Text URL Pasting', + control: { + type: 'boolean', + }, + }, + minHeightLines: { + name: 'Minimum Height (Lines)', + control: { + type: 'number', + }, + }, + maxHeightLines: { + name: 'Maximum Height (Lines)', + control: { + type: 'number', + }, + }, + hideLabel: { + name: 'Hide Label', + control: { + type: 'boolean', + }, + }, + required: { + name: 'Required', + control: { + type: 'boolean', + }, + }, + fileUploadsEnabled: { + name: 'Enable File Uploads', + control: { + type: 'boolean', + }, + }, + savedRepliesEnabled: { + name: 'Enable Saved Replies', + control: { + type: 'boolean', + }, + }, + onSubmit: { + name: 'onSubmit', + action: 'submitted', + }, + onDiffClick: { + name: 'onDiffClick', + action: 'diff-clicked', + }, + onFooterClick: { + name: 'onFooterClick', + action: 'footer-clicked', + }, + }, +} + +export default meta + +type ArgProps = { + disabled: boolean + fullHeight: boolean + monospace: boolean + minHeightLines: number + maxHeightLines: number + hideLabel: boolean + required: boolean + fileUploadsEnabled: boolean + savedRepliesEnabled: boolean + pasteUrlsAsPlainText: boolean + onSubmit: () => void + onDiffClick: () => void + onFooterClick: () => void +} + +const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +const fakeFileUrl = (file: File) => `https://image-store.example/file/${encodeURIComponent(file.name)}` + +const mentionables: Mentionable[] = [ + {identifier: 'monalisa', description: 'Monalisa Octocat'}, + {identifier: 'github', description: 'GitHub'}, + {identifier: 'primer', description: 'Primer'}, +] + +const emojis: Emoji[] = [ + {name: '+1', character: '👍'}, + {name: '-1', character: '👎'}, + {name: 'heart', character: '❤️'}, + {name: 'wave', character: '👋'}, + {name: 'raised_hands', character: '🙌'}, + {name: 'pray', character: '🙏'}, + {name: 'clap', character: '👏'}, + {name: 'ok_hand', character: '👌'}, + {name: 'point_up', character: '☝️'}, + {name: 'point_down', character: '👇'}, + {name: 'point_left', character: '👈'}, + {name: 'point_right', character: '👉'}, + {name: 'raised_hand', character: '✋'}, + {name: 'thumbsup', character: '👍'}, + {name: 'thumbsdown', character: '👎'}, + {name: 'octocat', url: 'https://github.githubassets.com/images/icons/emoji/octocat.png'}, +] + +const references: Reference[] = [ + {id: '1', titleText: 'Add logging functionality', titleHtml: 'Add logging functionality'}, + { + id: '2', + titleText: 'Error: `Failed to install` when installing', + titleHtml: 'Error: Failed to install when installing', + }, + {id: '3', titleText: 'Add error-handling functionality', titleHtml: 'Add error-handling functionality'}, +] + +const savedReplies: SavedReply[] = [ + {name: 'Duplicate', content: 'Duplicate of #'}, + {name: 'Welcome', content: 'Welcome to the project!\n\nPlease be sure to read the contributor guidelines.'}, + {name: 'Thanks', content: 'Thanks for your contribution!'}, + { + name: 'Long Lorem Ipsum', + content: + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam sodales ligula commodo ex venenatis molestie. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae; Curabitur vulputate elementum dolor ac sollicitudin. Duis tellus quam, hendrerit sit amet metus quis, pharetra consectetur eros. Duis purus justo, convallis nec velit nec, feugiat pharetra nibh. Aenean vulputate urna sollicitudin vehicula fermentum. Vestibulum semper iaculis metus, quis ullamcorper dui feugiat a. Donec nulla sapien, tincidunt ut arcu sit amet, ultrices fringilla massa. Integer ac justo lacus.\n\nFusce sed pharetra sem. Nulla rutrum turpis magna, sit amet sodales dui vehicula in. Cras lacinia, dui sit amet dictum lobortis, arcu erat semper lectus, placerat accumsan diam dolor nec quam. Vivamus accumsan ut magna eget maximus. Integer scelerisque justo et quam pharetra, nec placerat nibh auctor. Vestibulum cursus, mauris id euismod convallis, justo sapien faucibus dolor, nec dictum erat urna at velit. Quisque egestas massa eget odio consectetur vehicula. Aliquam a imperdiet lacus, eu facilisis mauris. Etiam tempor neque vitae erat elementum bibendum. Fusce ultricies nunc tortor.\n\nQuisque in posuere sapien. Nulla ornare sagittis tellus eu laoreet. Sed molestie sem in turpis blandit pretium. Vivamus gravida dui id gravida aliquam. Vestibulum vestibulum, justo vitae cursus mattis, urna mauris pulvinar dolor, eu suscipit magna libero eget diam. Praesent id rutrum libero, a feugiat nulla. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Curabitur ornare libero id augue fringilla maximus sed sed ante. Quisque finibus accumsan lorem ut lobortis. Maecenas lobortis lacus sed mattis rutrum. Aliquam a mi sodales, blandit nisi ut, volutpat ex. Duis tristique, erat quis fermentum ultricies, leo ipsum placerat nunc, eu aliquam nibh mauris vitae lectus. Proin vitae tellus nec lorem vulputate faucibus. In hac habitasse platea dictumst. Suspendisse dictum odio in faucibus mattis.', + }, +] + +const onUploadFile = async (file: File) => { + const wait = 0.0002 * file.size + 500 + await delay(wait / 2) + // to demo file rejections: + if (file.name.toLowerCase().startsWith('a')) throw new Error("Rejected file for starting with the letter 'a'") + // 0.5 - 5 seconds depending on file size up to about 20 MB + await delay(wait / 2) + return {file, url: fakeFileUrl(file)} +} + +const renderPreview = async () => { + await delay(500) + return 'Previewing Markdown is not supported in this example.' +} + +export const CustomToolbar = ({ + disabled, + fullHeight, + monospace, + minHeightLines, + maxHeightLines, + hideLabel, + required, + fileUploadsEnabled, + onSubmit, + onDiffClick, + savedRepliesEnabled, + pasteUrlsAsPlainText, +}: ArgProps) => { + const [value, setValue] = useState('') + + return ( + + + Markdown Editor Example + + + + + + +

Note: for demo purposes, files starting with "A" will be rejected.

+ + ) +} + +export const CustomFooter = ({ + disabled, + fullHeight, + monospace, + minHeightLines, + maxHeightLines, + hideLabel, + required, + fileUploadsEnabled, + onSubmit, + onFooterClick, + savedRepliesEnabled, + pasteUrlsAsPlainText, +}: ArgProps) => { + const [value, setValue] = useState('') + + return ( + + + Markdown Editor Example - Custom Footer + + + + Add Button + + + + setValue('')}> + Reset + + + Submit + + + + +

Note: for demo purposes, files starting with "A" will be rejected.

+ + ) +} + +export const CustomFooterActions = ({ + disabled, + fullHeight, + monospace, + minHeightLines, + maxHeightLines, + hideLabel, + required, + fileUploadsEnabled, + onSubmit, + savedRepliesEnabled, + pasteUrlsAsPlainText, +}: ArgProps) => { + const [value, setValue] = useState('') + + return ( + + + Markdown Editor Example + + + setValue('')}> + Reset + + + Submit + + + +

Note: for demo purposes, files starting with "A" will be rejected.

+ + ) +} + +function useLazySuggestions(suggestions: T[]) { + const promiseRef = useRef | null>(null) + + return () => { + // This simulates waiting to make an API request until the first time the suggestions are needed + // Then, once we have made the API request we keep returning the same Promise which will already + // be resolved with the cached data + if (!promiseRef.current) { + promiseRef.current = new Promise(resolve => { + setTimeout(() => resolve(suggestions), 500) + }) + } + + return promiseRef.current + } +} + +export const LazyLoadedSuggestions = ({ + disabled, + fullHeight, + monospace, + minHeightLines, + maxHeightLines, + hideLabel, + required, + fileUploadsEnabled, + onSubmit, + savedRepliesEnabled, + pasteUrlsAsPlainText, +}: ArgProps) => { + const [value, setValue] = useState('') + + const emojiSuggestions = useLazySuggestions(emojis) + const mentionSuggestions = useLazySuggestions(mentionables) + const referenceSuggestions = useLazySuggestions(references) + + return ( + + + Markdown Editor Example + +

Note: for demo purposes, files starting with "A" will be rejected.

+ + ) +} diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.stories.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.stories.tsx index ecfcca7871e..714a9736d80 100644 --- a/src/drafts/MarkdownEditor/MarkdownEditor.stories.tsx +++ b/src/drafts/MarkdownEditor/MarkdownEditor.stories.tsx @@ -1,6 +1,5 @@ -import {DiffIcon, PlusIcon} from '@primer/octicons-react' import React, {Meta} from '@storybook/react' -import {useRef, useState} from 'react' +import {useState} from 'react' import BaseStyles from '../../BaseStyles' import Box from '../../Box' import MarkdownEditor, {Emoji, Mentionable, Reference, SavedReply} from '.' @@ -207,112 +206,7 @@ const renderPreview = async () => { return 'Previewing Markdown is not supported in this example.' } -export const Default = ({ - disabled, - fullHeight, - monospace, - minHeightLines, - maxHeightLines, - hideLabel, - required, - fileUploadsEnabled, - onSubmit, - savedRepliesEnabled, - pasteUrlsAsPlainText, -}: ArgProps) => { - const [value, setValue] = useState('') - - return ( - <> - - Markdown Editor Example - -

Note: for demo purposes, files starting with "A" will be rejected.

- - ) -} - -export const CustomToolbar = ({ - disabled, - fullHeight, - monospace, - minHeightLines, - maxHeightLines, - hideLabel, - required, - fileUploadsEnabled, - onSubmit, - onDiffClick, - savedRepliesEnabled, - pasteUrlsAsPlainText, -}: ArgProps) => { - const [value, setValue] = useState('') - - return ( - <> - - Markdown Editor Example - - - - - - -

Note: for demo purposes, files starting with "A" will be rejected.

- - ) -} - -export const CustomFooter = ({ - disabled, - fullHeight, - monospace, - minHeightLines, - maxHeightLines, - hideLabel, - required, - fileUploadsEnabled, - onSubmit, - onFooterClick, - savedRepliesEnabled, - pasteUrlsAsPlainText, -}: ArgProps) => { +export const Default = () => { const [value, setValue] = useState('') return ( @@ -320,50 +214,18 @@ export const CustomFooter = ({ - Markdown Editor Example - Custom Footer - - - - Add Button - - - - setValue('')}> - Reset - - - Submit - - - + Markdown Editor Example

Note: for demo purposes, files starting with "A" will be rejected.

) } -export const CustomFooterActions = ({ +export const Playground = ({ disabled, fullHeight, monospace, @@ -395,79 +257,6 @@ export const CustomFooterActions = ({ emojiSuggestions={emojis} mentionSuggestions={mentionables} referenceSuggestions={references} - required={required} - savedReplies={savedRepliesEnabled ? savedReplies : undefined} - pasteUrlsAsPlainText={pasteUrlsAsPlainText} - > - Markdown Editor Example - - - setValue('')}> - Reset - - - Submit - - - -

Note: for demo purposes, files starting with "A" will be rejected.

- - ) -} - -function useLazySuggestions(suggestions: T[]) { - const promiseRef = useRef | null>(null) - - return () => { - // This simulates waiting to make an API request until the first time the suggestions are needed - // Then, once we have made the API request we keep returning the same Promise which will already - // be resolved with the cached data - if (!promiseRef.current) { - promiseRef.current = new Promise(resolve => { - setTimeout(() => resolve(suggestions), 500) - }) - } - - return promiseRef.current - } -} - -export const LazyLoadedSuggestions = ({ - disabled, - fullHeight, - monospace, - minHeightLines, - maxHeightLines, - hideLabel, - required, - fileUploadsEnabled, - onSubmit, - savedRepliesEnabled, - pasteUrlsAsPlainText, -}: ArgProps) => { - const [value, setValue] = useState('') - - const emojiSuggestions = useLazySuggestions(emojis) - const mentionSuggestions = useLazySuggestions(mentionables) - const referenceSuggestions = useLazySuggestions(references) - - return ( - <> - Date: Fri, 15 Sep 2023 00:00:28 -0500 Subject: [PATCH 03/44] fix(ButtonBase): remove leading, trailing props from being spread onto base button (#3744) Co-authored-by: Josh Black --- src/Button/ButtonBase.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Button/ButtonBase.tsx b/src/Button/ButtonBase.tsx index bf11edff9e2..6ae4ae4b274 100644 --- a/src/Button/ButtonBase.tsx +++ b/src/Button/ButtonBase.tsx @@ -11,6 +11,10 @@ import {defaultSxProp} from '../utils/defaultSxProp' const ButtonBase = forwardRef( ({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => { const { + leadingIcon, + leadingVisual, + trailingIcon, + trailingVisual, trailingAction: TrailingAction, icon: Icon, variant = 'default', @@ -19,8 +23,8 @@ const ButtonBase = forwardRef( block = false, ...rest } = props - const LeadingVisual = props.leadingVisual ?? props.leadingIcon - const TrailingVisual = props.trailingVisual ?? props.trailingIcon + const LeadingVisual = leadingVisual ?? leadingIcon + const TrailingVisual = trailingVisual ?? trailingIcon const innerRef = React.useRef(null) useRefObjectAsForwardedRef(forwardedRef, innerRef) From 0ff78aff689365b204d954f37453e5829efebc48 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 15 Sep 2023 10:39:00 -0500 Subject: [PATCH 04/44] docs(versioning): update docs to include landmark roles (#3743) * docs(versioning): update docs to include landmark roles * docs: update table with correct semver bump --------- Co-authored-by: Josh Black --- contributor-docs/versioning.md | 72 ++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/contributor-docs/versioning.md b/contributor-docs/versioning.md index adbee120bc7..61725b7f939 100644 --- a/contributor-docs/versioning.md +++ b/contributor-docs/versioning.md @@ -11,6 +11,8 @@ - [The type of a prop is broadened](#the-type-of-a-prop-is-broadened) - [The type of a prop is narrowed](#the-type-of-a-prop-is-narrowed) - [The `display` property used for the container of `children` is changed](#the-display-property-used-for-the-container-of-children-is-changed) + - [A component includes a landmark role](#a-component-includes-a-landmark-role) + - [A component no longer includes a landmark role](#a-component-no-longer-includes-a-landmark-role) @@ -40,22 +42,24 @@ For a full list of releases, visit our [releases](https://github.com/primer/reac ## Changes -| Category | Type of change | semver bump | -| :-------- | :-------------------------------------------------------------------------------------------------------------------------------------------- | :------------------ | -| Component | A component is added | `minor` | -| | A component is deprecated | `minor` | -| | A component is removed | `major` | -| Props | A prop is added | `minor` | -| | [The type of a prop is broadened](#the-type-of-a-prop-is-broadened) | `minor` | -| | [The type of a prop is narrowed](#the-type-of-a-prop-is-narrowed) | `major` | -| | A prop is deprecated | `minor` | -| | A prop is removed | `major` | -| Package | A dependency is added | `minor` | -| | A dependency is removed and it does not affect the public API of the package | `minor` | -| | A dependency is removed and it does affect the public API of the package | `major` | -| | The version of a dependency is increased to a newer minor or patch version | `minor` | -| | The version of a dependency is increased to a newer major version | `major` | -| CSS | [The `display` property used for the container of `children` is changed](#the-display-property-used-for-the-container-of-children-is-changed) | potentially `major` | +| Category | Type of change | semver bump | +| :------------ | :-------------------------------------------------------------------------------------------------------------------------------------------- | :------------------ | +| Component | A component is added | `minor` | +| | A component is deprecated | `minor` | +| | A component is removed | `major` | +| Props | A prop is added | `minor` | +| | [The type of a prop is broadened](#the-type-of-a-prop-is-broadened) | `minor` | +| | [The type of a prop is narrowed](#the-type-of-a-prop-is-narrowed) | `major` | +| | A prop is deprecated | `minor` | +| | A prop is removed | `major` | +| Package | A dependency is added | `minor` | +| | A dependency is removed and it does not affect the public API of the package | `minor` | +| | A dependency is removed and it does affect the public API of the package | `major` | +| | The version of a dependency is increased to a newer minor or patch version | `minor` | +| | The version of a dependency is increased to a newer major version | `major` | +| CSS | [The `display` property used for the container of `children` is changed](#the-display-property-used-for-the-container-of-children-is-changed) | potentially `major` | +| Accessibility | [A component includes a landmark role](#a-component-includes-a-landmark-role) | potentially `major` | +| | [A component no longer includes a landmark role](#a-component-no-longer-includes-a-landmark-role) | potentially `major` | ## Reference @@ -123,3 +127,39 @@ markup: In this situation, changing the layout of the `button` to `flex` would collapse the whitespace present between the text "Save" and "changes" and would be considered a breaking change. + +### A component includes a landmark role + +semver bump: potentially **major** + +The addition of certain [landmark](https://w3c.github.io/aria/#dfn-landmark) regions may be considered a breaking change. For example, if a component includes the `main` landmark role and another `main` element already exists then the product would violate the [`landmark-no-duplicate-main`](https://dequeuniversity.com/rules/axe/4.8/landmark-no-duplicate-main?product=RuleDescription) rule that is a part of axe. + +The addition of other landmark roles may be permissible in a `minor` release if +and only if the component is uniquely identifiable from other landmarks of the +same role. This is typically done through an explicit label on the landmark or +through an element which labels the landmark. + +| Role | semver bump | Description | +| :------------ | :------------------ | :----------------------------------------------------------------------------------------- | +| banner | potentially `major` | `major` if used as global site header, `minor` if used in sectioning elements | +| complementary | `minor` | | +| contentinfo | `major` | The implicit role of `