User mentions: add popover #24287

bluefuton · 2018-04-18T06:50:59Z

Follows on from #24194, which created the user mentions block.

This PR adds a popover for suggestions, based on the current TinyMCE implementation in client/components/tinymce/mentions.

To test

We're not seeking perfection with this one - smaller bugfixes and improvements can be created as followup issues. It's still using test data to populate usernames. As long as its basically functional and inserts usernames as expected, let's 🚢 and keep iterating on it.

Give it a spin at:

http://calypso.localhost:3000/devdocs/blocks/user-mentions

matticbot · 2018-04-18T06:51:00Z

Test live: https://calypso.live/?branch=add/reader/user-mentions-block-popover

bluefuton · 2018-04-18T21:57:27Z

Lint build failures should be fixed by #24308.

bluefuton · 2018-04-19T07:29:55Z

Todo:

allow popover position to be manually specified (new PR: Popover: allow custom position to be specified #24346) and update this PR after it lands
adjust position on resize
handle end of line oddities (popup appears at the start of next line)
hide popover arrow (in CSS, not with prop)
browser test in IE, Safari, FF

Possible improvements

move example input forwardRef into the HOC
find caret position of @ rather than using current caret position, and use that for popup position (seen when starting a new line or resizing) - getCaretPosition() can accept this position as an argument rather than selectionEnd
sometimes suggestion popup is not fully visible when near the edge of the viewport and also edge of the text input
improve behaviour when prepending an @ to an existing word (currently inserts before it, even if the following word is the same)
cursor jumps to next line briefly during insertion when using enter
textbox loses focus during insertion when not picking with keyboard
add RTL support

jsnmoon

I'm a bit out of my elements with this change, so I'll provide feedback where I can.

It's very convenient to have this be testable in /devdocs/blocks/user-mentions 👍

jsnmoon · 2018-04-27T21:30:31Z

client/blocks/user-mentions/with-user-mentions.jsx

-			// ];
+			//const { siteId } = this.props;
+			const { query, showPopover } = this.state;
+			const suggestions = [


It might be useful to specify this using defaultProp.suggestions. That way, it'll be easy to swap out the placeholder suggestions in the future.

That's a smart idea :)

jsnmoon · 2018-04-27T21:31:48Z

client/blocks/user-mentions/with-user-mentions.jsx

 		};

+		handleKeyUp = event => {


I'm unfamiliar with how this works - why do we handle user input in both handleKeyDown and handleKeyUp? Would handling it all in handleKeyUp lead to unexpected behavior?

keyDown can repeat on a long press, but keyUp only ever fires once, after the key has been released:

https://javascript.info/keyboard-events

If you long press an up/down arrow, we'd want it to keep scrolling through the suggestions, hence it's handled by keyDown 🎹

jsnmoon · 2018-04-27T21:35:47Z

client/blocks/user-mentions/docs/example-input.jsx


 /**
 * Internal dependencies
 */
 import ExampleInput from './example-input';
 import withUserMentions from '../with-user-mentions';

-const UserMentionsExampleInput = ( { onKeyPress } ) => <textarea onKeyPress={ onKeyPress } />;
+// @todo Move ref forwarding to the HOC
+const UserMentionsExampleInput = React.forwardRef( ( props, ref ) => (


Interesting, this is the first time I've seen someone use forwardRef.

It's handy! I need access to the DOM node in the child component. I'm hoping to move the forwardRef into the higher-order component later: #24518.

Ah, gotcha; I think I understand this better now.

jsnmoon · 2018-04-27T21:37:00Z

client/blocks/user-mentions/with-user-mentions.jsx

+		constructor( props ) {
+			super( props );
+			// create a ref to store the textarea DOM element
+			this.textInput = React.createRef();


Does createRef mark the textInput variable as a React reference?

It does indeed.

https://reactjs.org/docs/refs-and-the-dom.html

Oh! It's a pattern to deprecate callback refs. Got it :)

jsnmoon

Looks like a good place to start iterating from; I left some comments inline that you might find useful.

Something worth pointing out: I noticed a bug where a random rectangle would render on top of the textarea upon clicking on the textarea for the first time:

I wouldn't consider it a blocker for this PR, but figured you'd like to keep track of it.

jsnmoon · 2018-04-30T21:23:29Z

client/blocks/user-mentions/with-user-mentions.jsx

+				top: nodeRect.top + caretPosition.top + 28,
+			};
+
+			console.log( position ); // eslint-disable-line no-console


Perhaps a good time to use debug( position );?

jsnmoon · 2018-04-30T21:25:44Z

client/blocks/user-mentions/with-user-mentions.jsx

+			return suggestions.slice( 0, 10 );
+		}
+
+		insertSuggestion = ( { user_login: userLogin } ) => {


Maybe autocompleteSuggestion would be easier to understand?

I agree, but will stick with insertSuggestion for the moment to be consistent with Editor mentions.

bluefuton · 2018-04-30T22:30:43Z

Thanks for the 👀 @jsnmoon!

Add textarea-caret package User mentions changes Update shrinkwrap Add styles back to _components

bluefuton added [Status] In Progress [Feature] Reader The reader site on Calypso. labels Apr 18, 2018

bluefuton self-assigned this Apr 18, 2018

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from 793e4db to 47463f8 Compare April 19, 2018 02:54

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from 300be0e to 21e4f09 Compare April 20, 2018 00:41

bluefuton added this to the Reader: add @mentions milestone Apr 20, 2018

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from c02eb7b to 926023c Compare April 24, 2018 01:22

bluefuton mentioned this pull request Apr 24, 2018

Rename user-mention-suggestions block to user-mentions #24418

Merged

bluefuton force-pushed the add/reader/user-mentions-block-popover branch 2 times, most recently from 316e981 to 511c64e Compare April 26, 2018 01:01

This was referenced Apr 27, 2018

User mentions: add RTL support #24516

Open

User mentions: move React.forwardRef into the higher-order component #24518

Closed

bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Apr 27, 2018

bluefuton requested review from westi and jsnmoon April 27, 2018 04:03

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from 7548810 to 13b1fd5 Compare April 27, 2018 04:17

jsnmoon reviewed Apr 27, 2018

View reviewed changes

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from 0593bd9 to 9232dfe Compare April 30, 2018 00:52

bluefuton mentioned this pull request Apr 30, 2018

User mentions: connect to real suggestions #24558

Merged

jsnmoon approved these changes Apr 30, 2018

View reviewed changes

bluefuton added 9 commits May 2, 2018 09:40

Rename HoC

db3f4d5

Add textarea-caret package User mentions changes Update shrinkwrap Add styles back to _components

Use new popover custom position

eb656e2

Adapt popover position on resize

b47239a

Fix new line positioning

f4f8319

Drop redundant styles

544c271

Update shrinkwrap

50d4563

Move sample suggested users to props

272c15f

Update shrinkwrap

dcf8e44

Update shrinkwrap

76015dd

bluefuton force-pushed the add/reader/user-mentions-block-popover branch from 9232dfe to 76015dd Compare May 1, 2018 21:48

bluefuton merged commit b9a5e74 into master May 1, 2018

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 1, 2018

bluefuton deleted the add/reader/user-mentions-block-popover branch May 13, 2018 20:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User mentions: add popover #24287

User mentions: add popover #24287

bluefuton commented Apr 18, 2018 •

edited

Loading

matticbot commented Apr 18, 2018

bluefuton commented Apr 18, 2018

bluefuton commented Apr 19, 2018 •

edited

Loading

jsnmoon left a comment

jsnmoon Apr 27, 2018

bluefuton Apr 30, 2018

jsnmoon Apr 27, 2018

bluefuton Apr 30, 2018

jsnmoon Apr 27, 2018

bluefuton Apr 30, 2018

jsnmoon Apr 30, 2018

jsnmoon Apr 27, 2018

bluefuton Apr 30, 2018

jsnmoon Apr 30, 2018 •

edited

Loading

jsnmoon left a comment

jsnmoon Apr 30, 2018

jsnmoon Apr 30, 2018

bluefuton May 1, 2018

bluefuton commented Apr 30, 2018

User mentions: add popover #24287

User mentions: add popover #24287

Conversation

bluefuton commented Apr 18, 2018 • edited Loading

To test

matticbot commented Apr 18, 2018

bluefuton commented Apr 18, 2018

bluefuton commented Apr 19, 2018 • edited Loading

jsnmoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsnmoon Apr 30, 2018 • edited Loading

Choose a reason for hiding this comment

jsnmoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluefuton commented Apr 30, 2018

bluefuton commented Apr 18, 2018 •

edited

Loading

bluefuton commented Apr 19, 2018 •

edited

Loading

jsnmoon Apr 30, 2018 •

edited

Loading