feat: Upstream Trix/RichTextField. #90
Conversation
stephenh
requested review from
bdow,
KoltonG,
ksk5280 and
lucyconklin
as code owners
stephenh
changed the title
src/components/RichTextEditor.tsx
Outdated
| 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.
We still have an issue with vitejs where these "css imports that are done in a dependency" don't work :-/
src/components/RichTextEditor.tsx
Outdated
| @@ -0,0 +1,152 @@ | |||
| import { Global } from "@emotion/react"; | |||
| import * as React from "react"; | |||
There was a problem hiding this comment.
Question: Is this still required with our new setup in Beam?
There was a problem hiding this comment.
It's only for the React.createElement line down below which is how the original code that we copy/pasted created the <trix-editor> custom HTML element / web component thingy.
There was a problem hiding this comment.
Since the line below is already importing React what about we merge them and/or take out the createElement via a named export on line 3?
There was a problem hiding this comment.
Oh sure, I was too wrote-Java-for-a-decade thinking that React.createElement was "a static method on the React class", but we can import { createElement, ... }. Yep, changed.
src/components/RichTextEditor.tsx
Outdated
| import Tribute from "tributejs"; | ||
| import "tributejs/dist/tribute.css"; | ||
| import "trix/dist/trix"; | ||
| import "trix/dist/trix.css"; |
|
|
||
| export default { | ||
| component: RichTextEditor, | ||
| title: "Components/Rich Text Editor", |
There was a problem hiding this comment.
| title: "Components/Rich Text Editor", | |
| title: "Components/Rich Text Editors", |
Adding this will merge the parent story name and the child story into one.
There was a problem hiding this comment.
Oh, I get why you were doing this now. Cool, will change.
There was a problem hiding this comment.
Yeah! For single stories I usually opt for the singular title and do the as approach like you saw before
There was a problem hiding this comment.
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.
src/components/RichTextEditor.tsx
Outdated
| {/* TODO: Not sure what to pass to labelProps. */} | ||
| {label && <Label labelProps={{}} label={label} />} | ||
| <div css={trixCssOverrides}> | ||
| {React.createElement("trix-editor", { |
There was a problem hiding this comment.
Curious about this approach vs <TrixEditor /> usage (if I am understanding this right)?
There was a problem hiding this comment.
The <TrixEditor /> in BP is actually our own src/components/Trix.sx that is doing this same createElement(trix-editor) line. (Which is confusing, the <TrixEditor /> doesn't "look like our thing". I renamed it here to <RichTextField /> which I think will be less confusing.)
src/components/RichTextEditor.tsx
Outdated
| return ( | ||
| <div css={Css.w100.maxw("550px").$}> | ||
| {/* TODO: Not sure what to pass to labelProps. */} | ||
| {label && <Label labelProps={{}} label={label} />} |
There was a problem hiding this comment.
I think that is a React-Aria label thingy?
There was a problem hiding this comment.
Yeah, they come back from hooks like useTextField but I wasn't sure what look to use here. useTextField returns both labelProps and inputProps but the inputProps specifically want to be on an input type=text and not the input type=hidden. ...not really sure.
src/components/RichTextEditor.tsx
Outdated
| ); | ||
| } | ||
|
|
||
| function attachTributeJs(mergeTags: string[], editorElement: HTMLElement) { |
There was a problem hiding this comment.
Question: Wondering if we could return the tribute object and possibly add useEffect in the component above listening on the mergeTags and use https://www.npmjs.com/package/tributejs#updating-a-collection-with-new-data to real-time update the tags? Not sure if that was attempted before?
There was a problem hiding this comment.
Yeah, we could do that, just hasn't been necessary yet.
I copy/pasted this over and hopefully this will be fine for awhile, but it might be worth using a beam Menu and useOverlay to do this native and drop tribute all together at some point.
src/components/RichTextEditor.tsx
Outdated
|
|
||
| function attachTributeJs(mergeTags: string[], editorElement: HTMLElement) { | ||
| const values = mergeTags.map((value) => ({ value })); | ||
| const tribute = new Tribute({ |
There was a problem hiding this comment.
Pretty neat that you could include images here too! https://www.npmjs.com/package/tributejs#how-do-i-add-an-image-to-the-items-in-the-list but not sure how it will look 😋
src/components/RichTextEditor.tsx
Outdated
| } catch {} | ||
| } | ||
|
|
||
| const trixCssOverrides = { |
## [1.32.0](v1.31.0...v1.32.0) (2021-06-01) ### Features * Upstream Trix/RichTextField. ([#90](#90)) ([24fdcf5](24fdcf5))
|
🎉 This PR is included in version 1.32.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
~75% of is from my hack day where I needed a
BoundRichTextFieldin internal-frontend and so adapted/converted the BPTrixfrom the original class-based code we'd copy/pasted from a react-trix wrapper over into hooks.Also a first pass at Beam-ish styling, i.e. using our border radius, focus color, etc.