-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Upstream Trix/RichTextField. #90
Changes from 1 commit
ab3a82c
4ac2d4d
1ff0103
691311a
69ec9c0
a3a0602
fedf3ee
454282f
ba67a5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { Meta } from "@storybook/react"; | ||
import { useState } from "react"; | ||
import { RichTextEditor } from "src/components/RichTextEditor"; | ||
|
||
export default { | ||
component: RichTextEditor, | ||
title: "Components/Rich Text Editor", | ||
} as Meta; | ||
|
||
export function RichTextEditors() { | ||
return <TestEditor />; | ||
} | ||
|
||
function TestEditor() { | ||
const [value, setValue] = useState(""); | ||
return ( | ||
<> | ||
<RichTextEditor label="Comment" value={value} onChange={setValue} mergeTags={["foo", "bar", "zaz"]} /> | ||
value: {value} | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
import { Global } from "@emotion/react"; | ||
import { useTextField } from "@react-aria/textfield"; | ||
import * as React from "react"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Is this still required with our new setup in Beam? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's only for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the line below is already importing React what about we merge them and/or take out the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh sure, I was too wrote-Java-for-a-decade thinking that |
||
import { ChangeEvent, useEffect, useRef } from "react"; | ||
import { useId } from "react-aria"; | ||
import { Label } from "src/components/Label"; | ||
import { Css, Palette } from "src/Css"; | ||
import Tribute from "tributejs"; | ||
import "tributejs/dist/tribute.css"; | ||
import "trix/dist/trix"; | ||
import "trix/dist/trix.css"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still have an issue with vitejs where these "css imports that are done in a dependency" don't work :-/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surprising! |
||
|
||
export interface RichTextEditorProps { | ||
/** The initial html value to show in the trix editor. */ | ||
value: string; | ||
onChange: (html: string, text: string) => void; | ||
/** | ||
* A list of tags/names to show in a popup when the user `@`-s. | ||
* | ||
* Currently we don't support mergeTags being updated. | ||
*/ | ||
mergeTags?: string[]; | ||
label?: string; | ||
autoFocus?: boolean; | ||
placeholder?: string; | ||
} | ||
|
||
// trix + react-trix do not appear to have any typings, so roll with editor: any for now. | ||
type Editor = { | ||
expandSelectionInDirection?: (direction: "forward" | "backward") => void; | ||
insertString(s: string): void; | ||
loadHTML(html: string): void; | ||
}; | ||
|
||
function attachTributeJs(mergeTags: string[], editorElement: HTMLElement) { | ||
const values = mergeTags.map((value) => ({ value })); | ||
const tribute = new Tribute({ | ||
trigger: "@", | ||
lookup: "value", | ||
allowSpaces: true, | ||
/** {@link https://github.com/zurb/tribute#hide-menu-when-no-match-is-returned} */ | ||
noMatchTemplate: () => `<span style:"visibility: hidden;"></span>`, | ||
selectTemplate: ({ original: { value } }) => `<span style="color: ${Palette.LightBlue700};">@${value}</span>`, | ||
values, | ||
}); | ||
// In dev mode, this fails because jsdom doesn't support contentEditable. Note that | ||
// before create-react-app 4.x / a newer jsdom, the trix-initialize event wasn't | ||
// even fired during unit tests anyway. | ||
try { | ||
tribute.attach(editorElement!); | ||
} catch {} | ||
} | ||
|
||
/** | ||
* Glues together trix and tributejs to provide a simple rich text editor. | ||
* | ||
* See [trix]{@link https://github.com/basecamp/trix} and [tributejs]{@link https://github.com/zurb/tribute}. | ||
* */ | ||
export function RichTextEditor(props: RichTextEditorProps) { | ||
const { mergeTags, label, value, onChange } = props; | ||
const id = useId(); | ||
|
||
// We get a reference to the Editor instance after trix-init fires | ||
const editor = useRef<Editor | undefined>(undefined); | ||
|
||
// Disclaimer I'm kinda guessing at whether this aria setup is right | ||
const inputRef = useRef<HTMLInputElement | null>(null); | ||
const { labelProps, inputProps } = useTextField({ label }, inputRef); | ||
|
||
// Keep track of what we pass to onChange, so that we can make ourselves keep looking | ||
// like a controlled input, i.e. by only calling loadHTML if a new incoming `value` !== `currentHtml`, | ||
// otherwise we'll constantly call loadHTML and reset the user's cursor location. | ||
const currentHtml = useRef<string | undefined>(undefined); | ||
|
||
useEffect(() => { | ||
const editorElement = document.getElementById(`editor-${id}`); | ||
if (!editorElement) { | ||
throw new Error("editorElement not found"); | ||
} | ||
|
||
editor.current = (editorElement as any).editor; | ||
if (!editor.current) { | ||
throw new Error("editor not found"); | ||
} | ||
if (mergeTags !== undefined) { | ||
attachTributeJs(mergeTags, editorElement!); | ||
} | ||
// We have a 2nd useEffect to call loadHTML when value changes, but | ||
// we do this here b/c we assume the 2nd useEffect's initial evaluation | ||
// "missed" having editor.current set b/c trix-initialize hadn't fired. | ||
editor.current.loadHTML(value); | ||
|
||
function trixChange(e: ChangeEvent) { | ||
const { textContent, innerHTML } = e.target; | ||
currentHtml.current = innerHTML; | ||
onChange && onChange(innerHTML, textContent || ""); | ||
} | ||
|
||
editorElement.addEventListener("trix-change", trixChange as any, false); | ||
return () => { | ||
editorElement.removeEventListener("trix-change", trixChange as any); | ||
}; | ||
}, []); | ||
|
||
useEffect(() => { | ||
// If our value prop changes (without the change coming from us), reload it | ||
if (editor.current && value !== currentHtml.current) { | ||
editor.current.loadHTML(value); | ||
} | ||
}, [value]); | ||
|
||
const { placeholder, autoFocus } = props; | ||
|
||
return ( | ||
<div css={Css.w100.maxw("550px").$}> | ||
{label && <Label labelProps={labelProps} label={label} />} | ||
<div css={trixCssOverrides}> | ||
{React.createElement("trix-editor", { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious about this approach vs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
id: `editor-${id}`, | ||
input: `input-${id}`, | ||
...(autoFocus ? { autoFocus } : {}), | ||
...(placeholder ? { placeholder } : {}), | ||
})} | ||
<input type="hidden" ref={inputRef} id={`input-${id}`} value={value} /> | ||
</div> | ||
<Global styles={[tributeOverrides]} /> | ||
</div> | ||
); | ||
} | ||
|
||
const trixCssOverrides = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 awesome! |
||
...Css.relative.add({ wordBreak: "break-word" }).$, | ||
// Put the toolbar on the bottom | ||
...Css.df.flexColumnReverse.childGap1.$, | ||
// Some basic copy/paste from TextFieldBase | ||
"& trix-editor": Css.bgWhite.sm.gray900.br4.bGray300.$, | ||
"& trix-editor:focus": Css.bLightBlue700.$, | ||
// Make the buttons closer to ours | ||
"& .trix-button": Css.bgWhite.sm.$, | ||
// We don't support file attachment yet, so hide that control for now. | ||
"& .trix-button-group--file-tools": Css.dn.$, | ||
// Other things that are unused and we want to hide | ||
"& .trix-button--icon-heading-1": Css.dn.$, | ||
"& .trix-button--icon-code": Css.dn.$, | ||
"& .trix-button--icon-quote": Css.dn.$, | ||
"& .trix-button--icon-increase-nesting-level": Css.dn.$, | ||
"& .trix-button--icon-decrease-nesting-level": Css.dn.$, | ||
"& .trix-button-group--history-tools": Css.dn.$, | ||
// Put back list styles that CssReset is probably too aggressive with | ||
"& ul": Css.ml2.add("listStyleType", "disc").$, | ||
"& ol": Css.ml2.add("listStyleType", "decimal").$, | ||
}; | ||
|
||
// Style the @ mention box | ||
const tributeOverrides = { | ||
".tribute-container": Css.add({ minWidth: "300px" }).$, | ||
".tribute-container > ul": Css.sm.bgWhite.ba.br4.bLightBlue700.overflowHidden.$, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8515,11 +8515,6 @@ he@^1.2.0: | |
resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f" | ||
integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw== | ||
|
||
hey-listen@^1.0.8: | ||
version "1.0.8" | ||
resolved "https://registry.yarnpkg.com/hey-listen/-/hey-listen-1.0.8.tgz#8e59561ff724908de1aa924ed6ecc84a56a9aa68" | ||
integrity sha512-COpmrF2NOg4TBWUJ5UVyaCU2A88wEMkUPK4hNqyCkqHbxT92BbvfjoSozkAIIm6XhicGlJHhFdullInrdhwU8Q== | ||
|
||
header-case@^2.0.4: | ||
version "2.0.4" | ||
resolved "https://registry.yarnpkg.com/header-case/-/header-case-2.0.4.tgz#5a42e63b55177349cf405beb8d775acabb92c063" | ||
|
@@ -8528,6 +8523,11 @@ header-case@^2.0.4: | |
capital-case "^1.0.4" | ||
tslib "^2.0.3" | ||
|
||
hey-listen@^1.0.8: | ||
version "1.0.8" | ||
resolved "https://registry.yarnpkg.com/hey-listen/-/hey-listen-1.0.8.tgz#8e59561ff724908de1aa924ed6ecc84a56a9aa68" | ||
integrity sha512-COpmrF2NOg4TBWUJ5UVyaCU2A88wEMkUPK4hNqyCkqHbxT92BbvfjoSozkAIIm6XhicGlJHhFdullInrdhwU8Q== | ||
|
||
highlight.js@^10.1.1, highlight.js@~10.7.0: | ||
version "10.7.2" | ||
resolved "https://registry.yarnpkg.com/highlight.js/-/highlight.js-10.7.2.tgz#89319b861edc66c48854ed1e6da21ea89f847360" | ||
|
@@ -14810,6 +14810,11 @@ treeverse@^1.0.4: | |
resolved "https://registry.yarnpkg.com/treeverse/-/treeverse-1.0.4.tgz#a6b0ebf98a1bca6846ddc7ecbc900df08cb9cd5f" | ||
integrity sha512-whw60l7r+8ZU8Tu/Uc2yxtc4ZTZbR/PF3u1IPNKGQ6p8EICLb3Z2lAgoqw9bqYd8IkgnsaOcLzYHFckjqNsf0g== | ||
|
||
tributejs@^5.1.3: | ||
version "5.1.3" | ||
resolved "https://registry.yarnpkg.com/tributejs/-/tributejs-5.1.3.tgz#980600fc72865be5868893078b4bfde721129eae" | ||
integrity sha512-B5CXihaVzXw+1UHhNFyAwUTMDk1EfoLP5Tj1VhD9yybZ1I8DZJEv8tZ1l0RJo0t0tk9ZhR8eG5tEsaCvRigmdQ== | ||
|
||
trim-newlines@^3.0.0: | ||
version "3.0.0" | ||
resolved "https://registry.yarnpkg.com/trim-newlines/-/trim-newlines-3.0.0.tgz#79726304a6a898aa8373427298d54c2ee8b1cb30" | ||
|
@@ -14830,6 +14835,11 @@ [email protected]: | |
resolved "https://registry.yarnpkg.com/trim/-/trim-0.0.1.tgz#5858547f6b290757ee95cccc666fb50084c460dd" | ||
integrity sha1-WFhUf2spB1fulczMZm+1AITEYN0= | ||
|
||
trix@^1.3.1: | ||
version "1.3.1" | ||
resolved "https://registry.yarnpkg.com/trix/-/trix-1.3.1.tgz#ccce8d9e72bf0fe70c8c019ff558c70266f8d857" | ||
integrity sha512-BbH6mb6gk+AV4f2as38mP6Ucc1LE3OD6XxkZnAgPIduWXYtvg2mI3cZhIZSLqmMh9OITEpOBCCk88IVmyjU7bA== | ||
|
||
trough@^1.0.0: | ||
version "1.0.5" | ||
resolved "https://registry.yarnpkg.com/trough/-/trough-1.0.5.tgz#b8b639cefad7d0bb2abd37d433ff8293efa5f406" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this will merge the parent story name and the child story into one.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I get why you were doing this now. Cool, will change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! For single stories I usually opt for the singular title and do the
as
approach like you saw beforeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wish they would base this "merge parent story and child story" behavior off of "there is only 1 export'd function" instead of "the function name matches the file name", given that we will essentially always a conflict between the component-under-test name and that story file name.