Add optimistic rendering to article reactions

[gatlee]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Documentation Update

Description

Added optimistic rendering.

I am quite new to to this so I'm not sure how to write tests for it. If any tests should be written I can try to.

Additionally, ESLint tests did not pass after the commit. However, before I started work on the commit, it did not pass either. I have tried to the best of my ability to follow code guidelines in the areas I modified but have not refactored or cleaned up the other parts of the code.

Any suggestions are welcome!
Thanks
Gatlee Kaw

Related Tickets & Documents

#443

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Added to documentation?

• docs.dev.to
• readme
• no documentation needed

[optional] What gif best describes this PR or how it makes you feel?

[CLAassistant]

All committers have signed the CLA.

[gatlee]

I noticed that there is a version of the reaction buttons implemented in Preact which already has optimistic rendering. Is the plan to migrate that component over to that?

[benhalpern]

@gatlee yeah, that is the plan. But I could see us merging this in the meantime.

Since you're already engaged in this, would you be interested in attempting to rewrite the whole article reactions area as a preact component?

It's not an immediate team priority, so we'll review and likely merge the current change unless you want to jump the line and write the preact version. Let me know what your capacity for this is.

[gatlee]

Awesome! I look forward to the feedback.

Also, I'd be happy to work on a preact version of the component. I believe I'd be capable of doing so. I'll get working on it soon!

[gatlee]

Did a minor update to the code to revert the optimistic render on error. Not entirely sure why such a minor change resulted in a test failure however.

[gatlee]

@benhalpern On a side note: I haven't been able to work much on porting it to Preact since I've been quite busy. It might be better to review and merge this since in the meantime I probably won't have time to finish the port very soon

I had a small technical question too. So far I've added a Reactions.jsx which loads with a javascript_pack_tag

Inside the Reactions.jsx I have this.

document.addEventListener('DOMContentLoaded', () => {
  const root = document.getElementById('article-reaction-actions');
  const child = document.getElementById('article-reaction-child');
  render(<Reactions />, root, child);
});

Which only renders my custom Reactions element when the page is refreshed but is not triggered when the user clicks an article (presumably because the DOM isn't reloaded when opening an article). I was wondering if you had some idea of what the best way would be to get the render to occur whenever an article is opened

[maestromac]

Hey @gatlee , where did you place the javascript_pack_tag? Here my guess for why your Preact component isn't getting triggered:

We use a modified version of instantclick, which changes the way site navigation works. If you place the javascript_pack_tag file in application.html.erb, your Reaction.jsx will only run once and won't detect upcoming DOM changes because instantClick won't fire DOMContentLoaded state (I'm quite certain this is the case but I need to verify). You would have to place the javascript_pack_tag in the view that will definitely get loaded by instantClick.

No need to stress this, as there are probably better ways for this to be handled.

[gatlee]

The javascript_pack_tag is in the _actions.html.erb file which loads each time so that wasn't the problem.

I found the proper event listener to use for InstantClick here which seems to work correctly. I should be good to go ahead now. Thanks for the help!

[maestromac]

LGTM and thanks for this PR! Sorry it took us this long to review this. I went ahead and resolved the merge conflict.

[gatlee]

No worries. Glad I could contribute!