Page MenuHomePhabricator

TalkSectionOverlay should use the Overlay.make pattern
Open, Stalled, MediumPublic


Let's complete the migration of talk overlays with our new patterns! Hopefully, this should be a simpler migration applying everything we've learned so far. This should prove these migrations are getting easier.

The TalkSectionOverlay is all that remains.
It can be viewed if logged in here:

It looks like this:

The new Overlay will contain 2 elements:

  • The discussion (asynchronously loaded)
  • The reply form

We will also take the time to improve the reply form. Currently, if you type text into the reply form and click back you lose your reply. Let's add a confirmation step using the onBeforeExit Overlay callback and add error handling for when the reply form is unable to save.

acceptance criteria

  • If I try and exit the overlay while the reply form has content, I am asked to confirm
  • If I hit save and the post fails, I should see an error notification asking me to try again.
  • When hitting save and the save being successful, the overlay is refreshed displaying the new content

Developer notes

  • OverlayManager.replaceCurrent can be used to make the overlay refresh after a save
  • Use promisedView pattern for obtaining the current talk content.
  • Use onBeforeExit parameter of Overlay to prevent exiting the talk overlay

You may want to do this in at least 3 patches

  1. port the overlay to the new patterns
  2. Add onBeforeExit support and disable reply button when there is no content in the textarea
  3. Add error handling for reply form

QA steps

Test on the beta cluster.

  • Confirm the talk topic shows!
  • It should not be possible to send a reply if the textarea is empty.
  • Make your connection offline, when entering text into the reply form and hitting save, the spinner should show momentarily and the form should restore allowing the user to hit save again.
  • When save is clicked, the form should get replaced with a spinner and when the save completes the newly created reply should display above the form! Yay!
  • All replies should automatically be signed. Check this by adding "~~~~" and omitting those special characters and confirming the newly created reply has your username at the end of it!

Sign off steps

  • A super happy dev time topic is added to discuss whether this was as easy as expected and if not, what else we need to think about.
  • Check T219388 is indeed resolved and resolve it.

Event Timeline

Jdlrobson created this task.
Jdlrobson moved this task from Incoming to Upcoming on the Readers-Web-Backlog board.
ovasileva set the point value for this task to 5.Mar 13 2019, 4:13 PM

I don't think we should take on more of these tasks until the existing ones in flight have been done

Jdlrobson changed the task status from Open to Stalled.Apr 3 2019, 3:23 AM
Jdlrobson removed the point value for this task.

I think this is still ready to go, but I've reset the points and am stalling this on T217102. Whether we choose to work on this is the only question here! :)

This overlay has been removed from the codebase.

Whoops. This overlay is still around - my bad :)

Jdlrobson lowered the priority of this task from High to Medium.Nov 6 2019, 7:04 PM
Jdlrobson changed the task status from Open to Stalled.May 4 2021, 9:10 PM

Stalled per T278590