Add type ahead to mentions by dan-weaver · Pull Request #3793 · withspectrum/spectrum

dan-weaver · 2018-08-17T20:58:59Z

Status

WIP
Ready for review
Needs testing

Deploy after merge (delete what needn't be deployed)
TODO: Not sure yet

api
hyperion (frontend)
desktop
athena
vulcan
mercury
hermes
chronos
pluto
mobile

Release notes for users (delete if codebase-only change)

TODO

Related issues (delete if you don't know of any)
Closes #2649

prepend current thread participants onto search results
get typeahead working in new thread composer
clean up old mention decorator code
maintain backwards compat with old mentions text or otherwise create a db migration
don't break mobile rendering of mentions
make sure mentions still trigger appropriate serverside actions
disable enter key from triggering a submit when suggestions menu is in focus
tests!

Current UI:

Chat Input:

thread message:

WIP suggestion styles broke a whole lot of things cleanup logs

spectrum-bot · 2018-08-19T16:46:07Z

	Fails
🚫	1 console statement(s) left in src/components/chatInput/input.js.
🚫	These new files do not have Flow enabled: shared/clients/draft-js/mentionEntry/index.js src/components/chatInput/MentionSuggestions/Entry/Avatar/index.js src/components/chatInput/MentionSuggestions/Entry/defaultEntryComponent.js src/components/chatInput/MentionSuggestions/Entry/index.js src/components/chatInput/MentionSuggestions/index.js src/components/chatInput/hoc/withUserSearch.js src/components/chatInput/mentionPositionSuggestion.js

Generated by 🚫 dangerJS

dan-weaver · 2018-08-19T17:26:01Z

@mxstbr if you wouldn't mind taking a quick glance at this when you get a minute I'd love some (very) early feed back.

Some high level questions / notes:

I've stubbed out the search temporarily so that I can develop this locally without the algolia keys.

I'm using the draft-js mentions plugin here and it seems to have functionality that would replace the existing mentions decorators. I've found that the two don't play very well together, so so far I've just used the out of the box functionality from the draft-js plugin for decorating mentions in both the editor (just the chat input so far) as well as the redraft rendered messages. Is that an ok approach? Obviously there will be ramifications (the ones I could think of I've itemized above).

I was not planning on implementing this functionality in mobile for this PR. Would this feature existing in the web client but not the mobile apps just yet be tolerable? I do intend, however, to make sure that the existing mobile functionality stays intact!

This PR will make some slight changes to the way mentions are communicated between the web app and the api. More specifically, those mentions are now being created as an actual mention entity and saved as such on disk (I'm assuming, I haven't looked that far, but there doesn't appear to be any transformation of input in draft-js before being saved in the db).

To support older mentions for this change, would you advise maintaining support for both formats in code, or a db migration to update older mentions?

Still early, but let me know of any other concerns should you be able to spot them!

Thanks

mxstbr

Thanks so much for kicking this off! Got a couple of minor code nitpicks inline, don't worry too much about those just yet though.

Overall the approach to just use the draft-js-mentions-plugin makes sense. There's just a couple UX things I immediately noticed:

No loading indicator while searching (this should be an easy fix with the plugin)
Having to confirm a mention. I can't type Hey @mxstbr what's up? and expect it to mention @mxstbr—it won't be counted as a mention if I don't wait for the search to complete and then click it, which is annoying. I'd much rather it automatically links/mentions whatever I type (like it does now), but also gives me suggestions if I'm not sure how to spell a certain name. (not sure this can be fixed while using the plugin?)

Any clue how to change that second behavior?

mxstbr · 2018-08-20T15:03:30Z

src/components/chatInput/hoc/withUserSearch.js

@@ -0,0 +1,34 @@
+import { withStateHandlers, compose, withProps } from 'recompose';


nit: let's import these specifically, i.e.

import withStateHandlers from 'recompose/withStateHandlers'; import compose from 'recompose/compose'; // etc.

mxstbr · 2018-08-20T15:04:14Z

src/components/chatInput/hoc/withUserSearch.js

@@ -0,0 +1,34 @@
+import { withStateHandlers, compose, withProps } from 'recompose';
+import searchUsers from 'shared/graphql/queries/search/searchUsers';
+import { flatMap, map } from 'lodash';


nit: let's get rid of this lodash usage since we don't use it anywhere on the client and don't really need it here either:

data.search ? data.search.searchResultsConnection.edges.map(({ node }) => node) : [];

mxstbr · 2018-08-20T15:04:30Z

src/components/chatInput/index.js

 import { QuotedMessage as QuotedMessageComponent } from '../message/view';
 import type { Dispatch } from 'redux';
+import withUserSearch from './hoc/withUserSearch';
+import { consolidateStreamedStyles } from 'styled-components';


nit: don't need this here

dan-weaver · 2018-08-20T16:42:01Z

Having to confirm a mention. I can't type Hey @mxstbr what's up? and expect it to mention @mxstbr—it won't be counted as a mention if I don't wait for the search to complete and then click it, which is annoying. I'd much rather it automatically links/mentions whatever I type (like it does now), but also gives me suggestions if I'm not sure how to spell a certain name. (not sure this can be fixed while using the plugin?)

Yeah that's kinda a bummer. It does not appear to be supported by the plugin. @mxstbr Should this PR be able to move forward without that? If not, I suppose we can file an issue over at https://github.com/draft-js-plugins/draft-js-plugins and see what they think! To me it looks like a solution hacked outside of the plugin might be tough. 😞 Will look into it a bit further though.

brianlovin · 2018-08-20T16:57:37Z

It does not appear to be supported by the plugin.

Wow, that's surprising and really too bad. I agree with Max that the resulting experience should allow people to type usernames without having to go through any sort of dropdown and selection flow and we figure out how to tag them as mentions properly.

FWIW I'd be totally fine removing all mentions highlighting in the chat input itself. I know it's helpful to have it share a lot of the same formatting code as the thread composer, but if the chat input didn't highlight usernames and we only parse those mentions in the rendered sent message, that could work too?

dan-weaver · 2018-08-20T17:01:28Z

Yeah surprising enough that it feels like I'm missing something. Will keep digging. Would forking this plugin be a tolerable option for you?

mxstbr · 2018-08-20T17:30:51Z

Yeah totally fine with forking it!

dan-weaver · 2018-08-22T21:40:29Z

@mxstbr code is still a mess but I've made some modifications to override the behavior in regards to:

Having to confirm a mention. I can't type Hey @mxstbr what's up? and expect it to mention @mxstbr—it won't be counted as a mention if I don't wait for the search to complete and then click it, which is annoying. I'd much rather it automatically links/mentions whatever I type (like it does now), but also gives me suggestions if I'm not sure how to spell a certain name. (not sure this can be fixed while using the plugin?)

I think it's working as you envisioned now?
Should actually simplify things quite a bit. The mention plugin is basically just acting as a typeahead and it's opinions on how editor state gets updated are now removed.

dan-weaver · 2018-08-22T23:58:11Z

yikes.. Don't waste time looking at this yet :). found a couple annoying bugs!

mxstbr · 2018-08-24T14:16:47Z

@dan-weaver no worries, lmk when it's ready for review!

mxstbr · 2019-02-12T11:19:16Z

When we have moved our chat input to plaintext (#4472) we also finally implemented this without having to hack around any DraftJS stuff!

Thanks so much for your work on this @dan-weaver, really appreciate the contribution even though we did not end up merging it. This made building the current mention suggestions much easier! 🙏 🙏 🙏

WIP adding mentions

feabab5

WIP suggestion styles broke a whole lot of things cleanup logs

spectrum-bot bot added the WIP: Building label Aug 17, 2018

dan-weaver mentioned this pull request Aug 19, 2018

fixed mention regex to respect dots in usernames #3800

Merged

3 tasks

render mentions properly in thread messages

0c7a659

dan-weaver closed this Aug 19, 2018

dan-weaver force-pushed the alpha branch from feabab5 to 0b667f6 Compare August 19, 2018 16:44

dan-weaver reopened this Aug 19, 2018

remove todo

1ca372c

dan-weaver added 2 commits August 19, 2018 17:12

fix avatar fallback in MentionEntry

ba89ee1

add more test users to stubbed response

11a1963

mxstbr reviewed Aug 20, 2018

View reviewed changes

override default mention plugin behavior for state modification

3a9d004

fix conflict between mentions decorator and mentions plugin

1c72b93

mxstbr closed this Feb 12, 2019

		@@ -0,0 +1,34 @@
		import { withStateHandlers, compose, withProps } from 'recompose';

Conversation

dan-weaver commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current UI:

Uh oh!

spectrum-bot bot commented Aug 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-weaver commented Aug 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mxstbr left a comment

Choose a reason for hiding this comment

Uh oh!

mxstbr Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

mxstbr Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

mxstbr Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

dan-weaver commented Aug 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brianlovin commented Aug 20, 2018

Uh oh!

dan-weaver commented Aug 20, 2018

Uh oh!

mxstbr commented Aug 20, 2018

Uh oh!

dan-weaver commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-weaver commented Aug 22, 2018

Uh oh!

mxstbr commented Aug 24, 2018

Uh oh!

mxstbr commented Feb 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dan-weaver commented Aug 17, 2018 •

edited

Loading

spectrum-bot bot commented Aug 19, 2018 •

edited

Loading

dan-weaver commented Aug 19, 2018 •

edited

Loading

dan-weaver commented Aug 20, 2018 •

edited

Loading

dan-weaver commented Aug 22, 2018 •

edited

Loading