Move thread editor to plaintext #4564

mxstbr · 2019-01-23T12:23:54Z

Status

WIP
Ready for review
Needs testing

Deploy after merge (delete what needn't be deployed)

api
hyperion (frontend)

After reassessing the importance of this change due to customer feedback I decided to investigate this after all. From the first exploration it seems doable, with some caveats.

Todo

mxstbr · 2019-01-23T15:38:17Z

The hardest bits to figure out here are going to be images and embeds.

I think images we have to do GitHub-style "upload from client then input markdown link". /cc @brianlovin how would we best accomplish that? New uploadImage mutation?

I have no idea how to do the embeds, I think the choice is either to remove that functionality (while keeping existing embeds around, of course) or automatically "unfurling" a list of supported links into embeds (youtube, simplecast, soundcloud, etc) on the backend. Any thoughts?

spectrum-bot · 2019-01-23T15:41:40Z

	Warnings
⚠️	These modified files do not have Flow enabled: src/components/composer/style.js src/components/globals/index.js src/components/threadComposer/index.js src/components/threadComposer/style.js

Generated by 🚫 dangerJS

ThomasRoest · 2019-01-24T11:15:42Z

What are the problems with the current editor?

mxstbr · 2019-01-24T12:01:27Z

It is very buggy, lots of users constantly complain about it (see my recent blog post). We want to replace the WYSIWYG part with a plaintext markdown editor, which is then converted on the backend! (this should be mostly irrelevant for your work in #4575 I think?)

brianlovin · 2019-01-24T17:45:37Z

I think images we have to do GitHub-style "upload from client then input markdown link". /cc @brianlovin how would we best accomplish that? New uploadImage mutation?

Yeah, it might be worth poking around to see if other libs exist for this yet, otherwise we could try and figure out how GitHub handles this by just inserting placeholder markdown while waiting for the upload to return a url. The downside here would be that you create an upload every time, regardless of if the resulting url actually gets posted. But storage is cheap, so not a problem really right now.

I have no idea how to do the embeds, I think the choice is either to remove that functionality (while keeping existing embeds around, of course) or automatically "unfurling" a list of supported links into embeds (youtube, simplecast, soundcloud, etc) on the backend. Any thoughts?

I'm fine with removing functionality and getting really smart with link unfurling for popular services. I'd add Figma embeds and code sandbox urls to your list. I don't know how that would look in the backend (just inserting an iframe into the draftjs body?) but this feels like the best user experience to me personally.

mxstbr · 2019-01-24T20:10:37Z

The latest commit starts working on the client-side image uploads by adding an uploadImage mutation—doesn't quite work yet, but it'll get there I think.

The embed stuff sounds good to me too, inserting an embed into the DraftJS content seems simple enough to be feasible. I'll try making that happen after making sure the image uploads will work.

mxstbr · 2019-01-25T07:45:20Z

I have been battling around for hours with this image uploads stuff now, and can't seem to get it to work. When dropping an image on the new composer, (await file).stream always results in an undefined error on the server. Have you ever seen that @brianlovin, any hunches why that could be happening?

brianlovin · 2019-01-26T00:54:10Z

@mxstbr could you push up your latest work? Not sure what could be going on there. Do you have FILE_STORAGE=s3 set in your env? e.g. yarn run dev:api:s3 to ensure that uploads are hitting s3 and not your local directory?

mxstbr · 2019-01-27T15:53:27Z

That is not the issue, the image argument to the uploadImage mutation is just null even though it is provided from the frontend 😕 Latest commits are up!

mxstbr · 2019-01-30T08:17:33Z

Oh my god are you kidding me, did that fix it @brianlovin? 🤦‍♂️🤦‍♂️🤦‍♂️ I am such a dummy, sorry about all the struggles here. Cannot believe I spent three days messing around with this.

mxstbr · 2019-01-30T08:49:22Z

😍😍😍😍😍😍

A quite well unit tested util that parses supported URLs from text and returns their embed URLs. Currently supported services are Figma, Framer, Youtube, Vimeo, Codepen and Codesandbox, as well as support for generic <iframe>s

mxstbr · 2019-02-08T10:27:28Z

Todos

Fix overflow on /new/thread on desktop
Fix styling issues on /new/thread on mobile
Fix flow
Fix tests

ThomasRoest · 2019-02-08T10:51:49Z

this looks great!

brianlovin · 2019-02-08T21:59:08Z

@mxstbr @ThomasRoest this PR seriously makes me so happy. What a huge, massive, seriously big quality of life improvement to composition on Spectrum. I'm running this locally and just so stoked to ship this. Going to do a bunch of testing and make sure we can't suss out some lingering bugs.

brianlovin · 2019-02-08T22:08:53Z

@mxstbr found some bugs + notes:

links don't actually work. type [link](https://google.com) and it renders as a link, but the element isn't receiving the href attribute
how would you feel about also moving the edit experience to this modal? might reduce a ton of complexity from the edit screen/thread view; i'm a bit worried that the edit view has the old draftjs rich preview editor, which might introduce bugs cc @ThomasRoest
in the preview sometimes images have this draftjs focused state:

if you publish a thread with an image in this focused state, the final rendered thread will keep the focused styling

mxstbr · 2019-02-09T09:04:52Z

how would you guys feel about also moving the edit experience to this modal?

The "convert draftjs to markdown for editing" thing that we do for messages won't work for the complex styling of threads. For example, what do we do with automatic embeds?

If we do move editing to plain text we would need to store the raw markdown body on the server and send that down the wire.

I think that is more work than necessary for this v1, I would rather ship this and then tackle plaintext thread editing in a followup?

Noted on your other points, good catches! Will investigate on Monday

ThomasRoest · 2019-02-09T09:50:06Z

see components/rich-text-editor/Image.js for styling the image overlay / focus state in ImageContainer and ActiveOverlay

mxstbr · 2019-02-09T11:11:34Z

Yeah I think we can just entirely get rid of those focus states since they were only relevant during WYSIWYG editing. Want to submit a PR against this branch @ThomasRoest?

remove active overlay / focus state from image in composer

mxstbr · 2019-02-11T08:51:14Z

After thinking about this more, really the only thing the plaintext input does not account for is the automatic embeds. If we are fine with users not being able to remove them, we can move the editing experience to plaintext too.

That being said, it would require a lot more work to make plaintext editing work on both the back- and the frontend, so I would much rather ship this for now and then work on that afterwards if we are cool with that @brianlovin?

brianlovin · 2019-02-12T02:01:03Z

That being said, it would require a lot more work to make plaintext editing work on both the back- and the frontend, so I would much rather ship this for now and then work on that afterwards if we are cool with that @brianlovin?

Okay, I understand that's challenging. Since this is still a huge incremental improvement, let's 🚢 and figure out editing down the road. My 2 cents though is that I'm not worried about the user being able to edit the embeds after the fact.

mxstbr · 2019-02-12T09:11:35Z

Merging!!!!! 😱 🎉 🎉 🎉

Will get this live on alpha.

[WIP] Move thread editor to plaintext

be43fe4

spectrum-bot bot added the WIP: Building label Jan 23, 2019

Fix composer padding

9d55b1c

Start working on client-side image uploads

cc0a9a1

ThomasRoest mentioned this pull request Jan 24, 2019

WIP: combine / refactor threadcomposer #4575

Closed

8 tasks

mxstbr mentioned this pull request Jan 28, 2019

Workaround react-mentions bug with markup option #4588

Merged

3 tasks

ThomasRoest and others added 5 commits January 29, 2019 09:39

WIP: threadcomposer refactor first iteration

9d7211b

set styles based on isOpen prop

76166c8

update inserter icon design

7c442ab

Fix dev:api:s3 script to not show logging

8f8c505

Fix file uploads

0790a7b

Max Stoiber added 2 commits January 30, 2019 09:29

Merge branch 'alpha' into plaintext-thread-editor

34c9326

Nice client-side image uploads

d0ed008

Max Stoiber added 6 commits January 30, 2019 10:04

Fix flow

4d10e89

Remove console.log

97d3f97

Only add newline on file drop if required

b5741a3

Add parseEmbedsFromText util to API

bfad38b

A quite well unit tested util that parses supported URLs from text and returns their embed URLs. Currently supported services are Figma, Framer, Youtube, Vimeo, Codepen and Codesandbox, as well as support for generic <iframe>s

Automatically add embeds to published threads

5e2e03b

Handle embeds in thread editing

c6a69fc

Fix layout on /new/thread

cc72707

Max Stoiber added 3 commits February 8, 2019 12:34

Fix thread-composer data-cy attribute

c5f85a3

Fix e2e tests

a3979c5

Delete unused composer

9172f1a

brianlovin added 3 commits February 8, 2019 14:06

Fix actions padding

6fd1a45

Change placeholders

a424337

Return signed images so image previews work

9e769c2

remove active overlay / focus state from image in composer

a03f4b8

ThomasRoest mentioned this pull request Feb 10, 2019

remove active overlay / focus state from image in composer #4619

Merged

3 tasks

Merge pull request #4619 from ThomasRoest/plaintext-thread-editor

e966920

remove active overlay / focus state from image in composer

Max Stoiber added 3 commits February 11, 2019 10:30

Add support for link entites to links decorator

979034a

Fix markdown links in thread bodies

c2ba637

Fix rendering messages

aaefe6f

brianlovin approved these changes Feb 12, 2019

View reviewed changes

mxstbr merged commit 8e903bc into alpha Feb 12, 2019

mxstbr deleted the plaintext-thread-editor branch February 12, 2019 09:11

This was referenced Feb 12, 2019

add inline toolbar #3587

Closed

Media Providers #2861

Closed

Verify DraftJS inputs #3049

Closed

Thread renderer #4629

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move thread editor to plaintext #4564

Move thread editor to plaintext #4564

mxstbr commented Jan 23, 2019 •

edited

Loading

mxstbr commented Jan 23, 2019

spectrum-bot bot commented Jan 23, 2019 •

edited

Loading

ThomasRoest commented Jan 24, 2019

mxstbr commented Jan 24, 2019

brianlovin commented Jan 24, 2019

mxstbr commented Jan 24, 2019

mxstbr commented Jan 25, 2019

brianlovin commented Jan 26, 2019

mxstbr commented Jan 27, 2019 •

edited

Loading

mxstbr commented Jan 30, 2019

mxstbr commented Jan 30, 2019

mxstbr commented Feb 8, 2019 •

edited

Loading

ThomasRoest commented Feb 8, 2019 •

edited

Loading

brianlovin commented Feb 8, 2019

brianlovin commented Feb 8, 2019 •

edited by mxstbr

Loading

mxstbr commented Feb 9, 2019

ThomasRoest commented Feb 9, 2019

mxstbr commented Feb 9, 2019

mxstbr commented Feb 11, 2019 •

edited

Loading

brianlovin commented Feb 12, 2019

mxstbr commented Feb 12, 2019 •

edited

Loading

Move thread editor to plaintext #4564

Move thread editor to plaintext #4564

Conversation

mxstbr commented Jan 23, 2019 • edited Loading

mxstbr commented Jan 23, 2019

spectrum-bot bot commented Jan 23, 2019 • edited Loading

ThomasRoest commented Jan 24, 2019

mxstbr commented Jan 24, 2019

brianlovin commented Jan 24, 2019

mxstbr commented Jan 24, 2019

mxstbr commented Jan 25, 2019

brianlovin commented Jan 26, 2019

mxstbr commented Jan 27, 2019 • edited Loading

mxstbr commented Jan 30, 2019

mxstbr commented Jan 30, 2019

mxstbr commented Feb 8, 2019 • edited Loading

ThomasRoest commented Feb 8, 2019 • edited Loading

brianlovin commented Feb 8, 2019

brianlovin commented Feb 8, 2019 • edited by mxstbr Loading

mxstbr commented Feb 9, 2019

ThomasRoest commented Feb 9, 2019

mxstbr commented Feb 9, 2019

mxstbr commented Feb 11, 2019 • edited Loading

brianlovin commented Feb 12, 2019

mxstbr commented Feb 12, 2019 • edited Loading

mxstbr commented Jan 23, 2019 •

edited

Loading

spectrum-bot bot commented Jan 23, 2019 •

edited

Loading

mxstbr commented Jan 27, 2019 •

edited

Loading

mxstbr commented Feb 8, 2019 •

edited

Loading

ThomasRoest commented Feb 8, 2019 •

edited

Loading

brianlovin commented Feb 8, 2019 •

edited by mxstbr

Loading

mxstbr commented Feb 11, 2019 •

edited

Loading

mxstbr commented Feb 12, 2019 •

edited

Loading