Quote reply support on text-based posts and comments #526

SatsAllDay · 2023-09-27T01:44:26Z

Closes #516

Add support for a "Quote Reply" action on text-based posts and comments, available in the item info overflow menu.

Demo in 2 parts:
https://github.com/stackernews/stacker.news/assets/128755788/fd7ac334-492a-45b5-b7b1-e7a1545f4e28
https://github.com/stackernews/stacker.news/assets/128755788/8e226e09-b44f-4106-bb2e-1c27dd67a1ab

SatsAllDay · 2023-09-27T13:31:47Z

components/reply.js

+  const quoteReply = useCallback(() => {
+    if (!reply) {
+      setReply(true)
+    }
+    let updatedValue
+    if (formInnerRef.current && formInnerRef.current.values && !formInnerRef.current.values.text) {
+      updatedValue = quote(item.text)
+    } else if (formInnerRef.current?.values?.text) {
+      // append quote reply text if the input already has content
+      updatedValue = `${replyInput.current.value}\n${quote(item.text)}`
+    }
+    if (updatedValue) {
+      replyInput.current.value = updatedValue
+      formInnerRef.current.setValues({ text: updatedValue })
+      window.localStorage.setItem(`reply-${parentId}-text`, updatedValue)
+    }
+  }, [reply, item])
+  if (innerRef) {
+    innerRef.current = { quoteReply }


Apparently I just re-invented https://react.dev/reference/react/useImperativeHandle lol, I'll refactor to use that

SatsAllDay · 2023-09-27T19:53:47Z

...and here's quote reply selection. Note it's not going to give you full markdown quoted since it's based on the rendered text, not the raw markdown. I think that should suffice. You get full markdown quoted when you quote the entire item instead of a selection

cc @ekzyis

stacker.news.quote.reply.selection.mov

SatsAllDay · 2023-09-27T20:28:08Z

components/action-dropdown.js

@@ -8,7 +8,7 @@ export default function ActionDropdown ({ children }) {
  }
  return (
    <Dropdown className={`pointer ${styles.dropdown}`} as='span'>
-      <Dropdown.Toggle variant='success' as='a'>
+      <Dropdown.Toggle variant='success' as='a' onMouseDown={e => e.preventDefault()}>


This is to prevent clearing a text selection when opening the action dropdown menu, thus enabling quoting selections.

SatsAllDay · 2023-09-27T20:30:39Z

components/comment.js

@@ -197,7 +200,7 @@ export default function Comment ({
              />
              )
            : (
-              <div className={styles.text}>
+              <div className={styles.text} ref={contentContainerRef}>


Mark the DOM tree root where the item's text can be found, so we can check to see if a selection is made in the item when doing a quote reply, not just any selection in the entire page.

SatsAllDay · 2023-09-27T20:31:28Z

components/item-info.js

@@ -140,6 +142,8 @@ export default function ItemInfo ({
          !item.mine && !item.deletedAt && <DontLikeThisDropdownItem id={item.id} />}
        {item.mine && !item.position && !item.deletedAt &&
          <DeleteDropdownItem itemId={item.id} type={item.title ? 'post' : 'comment'} />}
+        {(item.parentId || item.text) && onQuoteReply &&


If it has a parent, or if it has text, and if we have a handler for the quote reply action, render the menu item.

SatsAllDay · 2023-09-27T20:31:56Z

components/reply.js

+  const replyInput = useRef(null)
+  const formInnerRef = useRef()
+  useImperativeHandle(ref, () => ({
+    quoteReply: () => {


An imperative quoteReply method to be triggered from above in the react tree.

SatsAllDay · 2023-09-27T20:32:16Z

components/reply.js

+      const selection = window.getSelection()
+      const selectedText = selection.isCollapsed ? undefined : selection.toString()
+      const isSelectedTextInTarget = contentContainerRef?.current?.contains(selection.anchorNode)
+      const textToQuote = isSelectedTextInTarget ? selectedText : item.text


Check to see what we should quote, the entire item, or a selection of text.

SatsAllDay · 2023-09-27T20:32:38Z

components/reply.js

@@ -96,7 +122,6 @@ export default function Reply ({ item, onSuccess, replyOpen, children, placehold
    setReply(replyOpen || false)
  }, [upsertComment, setReply, parentId])

-  const replyInput = useRef(null)


This was just moved up above for earlier use.

SatsAllDay · 2023-09-27T20:33:26Z

components/reply.js

-              </div>}
-          </Form>
-        </div>}
+      <div className={styles.reply} style={{ display: reply ? 'block' : 'none' }}>


Instead of conditionally rendering this div, I am toggling display CSS because I need the Form to exist even when the reply isn't shown, for the ref to be used.

huumn · 2023-09-28T00:25:04Z

omg this looks amazing! (haven't looked at the code yet)

maybe we should insert a newline after the quote or is that unergonomic for some reason?

SatsAllDay · 2023-09-28T00:30:05Z

omg this looks amazing! (haven't looked at the code yet)

Thank you! This is one of those areas where react does not shine IMO (imperative code), so while I don't like the code that much, it works better than trying to do it declaratively via props.

maybe we should insert a newline after the quote or is that unergonomic for some reason?

Yep, that's a good idea. That way any text you write doesn't get lumped into the quote automatically.

… just any selected text

SatsAllDay · 2023-09-29T12:44:28Z

maybe we should insert a newline after the quote or is that unergonomic for some reason?

Done

ekzyis

To summarize and it helps me to make sure I understand what's going on:

You create two refs in <Comment>. One to reference the reply (replyRef) and one to reference the container holding the content we are replying to (contentContainerRef).

The contentContainerRef is simply passed to a <div> inside <Comment>. Nothing more to follow there.

Both refs get passed to <Reply>.

In <Reply>, replyRef gets transformed using useImperativeHandle to only expose the quoteReply function. That way you can pass replyRef?.current?.quoteReply as quoteReply to <ItemInfo> in <Comment>. <ItemInfo> then renders conditionally the menu item for quote item.

contentContainerRef is used in quoteReply to detect if we selected something in the text we're replying to.

In <Reply> two more refs are created: replyInput and formInnerRef.

replyInput is passed to <MarkdownInput> which then references the <input> element (after going through <InputInner>, too). This one then allows us to reference the existing reply in quoteReply so we don't overwrite it.

formInnerRef is passed to <Form> and then to <Formik>. That one is required to actually update the text in quoteReply since it's controlled by formik.

LGTM! 😄

But while playing around, I think I found a way to make quote item disappear as a menu item ... then I started recording to reproduce it but I wasn't able to, lol

Will take another look later, but now I have gotten a good overview of what you did :)

SatsAllDay · 2023-09-29T13:53:29Z

You got it! As I mentioned above, the imperative stuff is kind of a mess. I wish this code was simpler but it just isn't. I welcome any simplification suggestions.

But while playing around, I think I found a way to make quote item disappear as a menu item ... then I started recording to reproduce it but I wasn't able to, lol

I have noticed it sometimes takes a moment for the "quote reply" action to show up when loading a page locally. I figured it was still loading the item into memory via GraphQL API call so that the conditional rendering logic could be satisfied, but I also was not able to reliably reproduce it. I figured it was a side-effect of my slow dev machine :P

ekzyis · 2023-09-29T14:17:11Z

I wish this code was simpler but it just isn't. I welcome any simplification suggestions.

I always wondered if we should make use of more local context. Afaik, we currently only use it on the global level to provide global context. But what if we use it to provide context for individual components, like to make Reply aware of what it's replying to etc.?

SatsAllDay · 2023-09-29T14:25:32Z

I actually didn't realize you could use the same context provider in a nested fashion. That could be useful in this context, no pun intended lol

SatsAllDay · 2023-09-29T14:48:21Z

Since the reply component already has the item to which it's replying as a prop, local context may only help with the "is the selection part of the item" aspect, which is cool, but we'll still need a ref for the DOM node, so I'm not sure it'll save us much in this case. Just my quick thinking, could be missing something...

huumn · 2023-09-29T18:06:09Z

Afaik, we currently only use it on the global level to provide global context.

fwiw we also use context to provide item roots to child comments

huumn · 2023-10-04T01:11:50Z

All this imperative stuff scares the crap out of me, but it's well written and super useful so we have to let it fly

I changed it to use onPointerDown to work on mobile. I also made it so that selecting text in the parent then hitting reply also quote replies.

Excellent work! 🚀🚀🚀

SatsAllDay · 2023-10-04T16:40:47Z

All this imperative stuff scares the crap out of me, but it's well written and super useful so we have to let it fly

I changed it to use onPointerDown to work on mobile. I also made it so that selecting text in the parent then hitting reply also quote replies.

Excellent work! 🚀🚀🚀

Thank you for the kind words 🙏 and thank you for the changes to enable mobile support + the other quote reply vector. Nice!

SatsAllDay force-pushed the quote-reply branch from 91881c2 to 6b4ff16 Compare September 27, 2023 01:47

SatsAllDay commented Sep 27, 2023

View reviewed changes

SatsAllDay mentioned this pull request Sep 27, 2023

Quote reply option #516

Closed

SatsAllDay force-pushed the quote-reply branch from 291ca7a to 2e7bda8 Compare September 27, 2023 19:55

SatsAllDay commented Sep 27, 2023

View reviewed changes

SatsAllDay marked this pull request as ready for review September 27, 2023 20:33

SatsAllDay added 5 commits September 28, 2023 20:57

Quote reply support on text-based posts and comments

3d9aa72

Clean up the onQuoteReply prop usage

b6845c5

Refactor to use useImperativeHandle for Reply

4b17c70

quote selected text if any, otherwise quote whole item

0eeb5aa

Only quote selected text if it's from the item we're replying to, not…

f75ec5c

… just any selected text

SatsAllDay force-pushed the quote-reply branch from 823f4d3 to f75ec5c Compare September 29, 2023 00:58

add trailing newline to copied text

d95e872

ekzyis reviewed Sep 29, 2023

View reviewed changes

huumn and others added 2 commits October 3, 2023 18:43

Merge branch 'master' into quote-reply

31c9570

onPointerDown for mobile, quote+reply quotes text

a6c8587

huumn merged commit f6141a6 into stackernews:master Oct 4, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quote reply support on text-based posts and comments #526

Quote reply support on text-based posts and comments #526

SatsAllDay commented Sep 27, 2023 •

edited

Loading

SatsAllDay Sep 27, 2023

SatsAllDay commented Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

SatsAllDay Sep 27, 2023

huumn commented Sep 28, 2023

SatsAllDay commented Sep 28, 2023

SatsAllDay commented Sep 29, 2023

ekzyis left a comment

SatsAllDay commented Sep 29, 2023

ekzyis commented Sep 29, 2023

SatsAllDay commented Sep 29, 2023

SatsAllDay commented Sep 29, 2023

huumn commented Sep 29, 2023

huumn commented Oct 4, 2023

SatsAllDay commented Oct 4, 2023

Quote reply support on text-based posts and comments #526

Quote reply support on text-based posts and comments #526

Conversation

SatsAllDay commented Sep 27, 2023 • edited Loading

Choose a reason for hiding this comment

SatsAllDay commented Sep 27, 2023

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

huumn commented Sep 28, 2023

SatsAllDay commented Sep 28, 2023

SatsAllDay commented Sep 29, 2023

ekzyis left a comment

Choose a reason for hiding this comment

SatsAllDay commented Sep 29, 2023

ekzyis commented Sep 29, 2023

SatsAllDay commented Sep 29, 2023

SatsAllDay commented Sep 29, 2023

huumn commented Sep 29, 2023

huumn commented Oct 4, 2023

SatsAllDay commented Oct 4, 2023

SatsAllDay commented Sep 27, 2023 •

edited

Loading