Help: Fix chat ended experience #1714

omarjackman · 2015-12-17T02:18:24Z

Fix "chat has ended" experience.

This pull request aims to fix the jarring experience a user might have when a chat has ended and there are no operators available.

Whats changed

Removed the operator availability notifications that currently show up inside of the olark chat widget.
Added calypso notifications to indicate olark operator availability when the chatbox is expanded or while viewing the contact form.
Show a notification within the olark widget immediately after an operator sends the !end command.
Allow the calypso notices to be dismissed.

How to test

Note: You must be in a group that has one operator

Navigate to http://calypso.localhost:3000/help/contact
Start a chat
Leave the contact form by clicking on any link in the sidebar.
Have the operator end the chat with the !end command.
Have the operator set them self as "Away" in olark.
Notice that the notification that operators went away now appears.
Have the operator set them self as "Available" in olark.
Notice that the notificaiton appears again saying that operators have returned.
Collapse the olark widget and repeat steps 5 and 7. Notice that no availability messages were displayed because the olark widget is collapsed.

Before

After

Fixes #1340
Fixes #1341
Fixes #1337

omarjackman · 2015-12-17T07:58:33Z

cc @mtias @dllh review please

dllh · 2015-12-17T15:01:00Z

I tested this a bit, and it's working well for me. Would rather have another set of eyes on the code, though it looked fine to my somewhat inexperienced eye.

dllh · 2015-12-17T18:45:46Z

I did some more testing and it still feels really good on the whole. I think it's a little weird still that in the case of no operators and and ended chat, we re-render the contact form with chat in the lower right and a notice rather than just showing the inline chat with some ui indicating that the chat is done (like what's displayed here), but I think that on the whole, this is a big improvement and we should try to get the code reviewed asap and get this merged.

omarjackman · 2015-12-17T19:15:33Z

I think it's a little weird still that in the case of no operators and and ended chat

@dllh this is fixed now.

omarjackman · 2015-12-17T19:16:03Z

cc @mtias @dmsnell review please

dmsnell · 2015-12-17T19:23:01Z

client/lib/olark-events/index.js

@@ -78,7 +78,7 @@ var OlarkEventEmitter = {
 	 */
 	olarkEventListener: function( event, ...eventArguments ) {
 		debug( 'Olark event: %s', event );
-		this.emit( event, eventArguments );
+		this.emit( event, ...eventArguments );


This is a 0.5 out of 10 as far as being an important critique, but is eventArguments a little verbose when something like args seems like it would convey the same meaning?

olarkApi( event, ( ...args ) => this.olarkEventListener( event, ...args ) ); … olarkEventListener( event, ...args ) { this.emit( event, ...args ); }

dmsnell · 2015-12-17T19:24:11Z

client/lib/olark-store/actions.js

+	setExpanded( isOlarkExpanded ) {
+		dispatcher.handleServerAction( {
+			isOlarkExpanded,
+			type: ActionTypes.OLARK_EXPANDED


This is slightly confusing. Maybe OLARK_SET_EXPANDED would be a clearer indication that the action is doing something.

dmsnell · 2015-12-17T19:35:27Z

@omarjackman I left a few suggestions. Happy to go for a second pass when you need it.

dllh · 2015-12-17T19:42:56Z

I think the fixes for the last case I mentioned are a big improvement in terms of the interaction. I do have one more bit of feedback for it, but I'd not block the PR for this (would instead see about improving in a future PR). Here's what I see when opers are available and the chat ends:

Looks great. If the oper then goes away, I see this:

It's definitely better in my opinion that jumping back to the contact form with the chat box at lower right, but it's a little weird that it now shows the different offline form and doesn't have info pre-filled even though I'm logged in. I know that this is something Olark does, so we may be very limited in what we can do to fix it. This feels to me like an improved experience, so I'd be in favor of landing with this and then later exploring our options to improve it further.

omarjackman · 2015-12-17T19:59:40Z

@dmsnell all comments addressed. More review please.

dmsnell · 2015-12-17T20:26:35Z

looks good on the code size @omarjackman. if there were one thing to mention it's that I probably wouldn't be declaring those new functions as such in the linear flow of the code as it is. I'd probably either create variables that are functions at the top of the scope or create those functions outside of the scope. it's strange to see a bunch of sequential code and then a function definition and then more sequential code

I did not test, so please merge at will as long as the testing is taken care of

… and discontinue showing these notices in the olark widget as visitor notices. Also keep chat inlined if chat ends while on the contact form with no available operators.

omarjackman · 2015-12-17T20:55:49Z

if there were one thing to mention it's that I probably wouldn't be declaring those new functions as such in the linear flow

@dmsnell I moved it out of the function.

omarjackman · 2015-12-17T21:42:36Z

@dmsnell @dllh can I git a 🚢 ?

dmsnell · 2015-12-17T22:12:35Z

🚢

mtias · 2015-12-17T22:26:38Z

client/lib/olark/index.js

+	showAvailabilityNotice() {
+		const noticeOptions = { showDismiss: true };
+		const { isOperatorAvailable, isUserEligible, isOlarkExpanded, isOlarkReady } = olarkStore.get();
+		const onEligibleContactForm = isUserEligible && window.location.pathname === '/help/contact';


This is not a blocker now, but I find it odd to have this tied to a url from this place — seems like the route should choose to display the notice, not the module look at the route.

In other words, this seems like controller's business.

…bility-notice Help: Fix chat ended experience

omarjackman added [Status] In Progress Live Chat [Feature Group] Support All things related to WordPress.com customer support. labels Dec 17, 2015

omarjackman self-assigned this Dec 17, 2015

omarjackman force-pushed the update/olark-operator-availability-notice branch 3 times, most recently from ce75090 to 7f250c2 Compare December 17, 2015 03:02

omarjackman mentioned this pull request Dec 17, 2015

Help: Show chat notification when chat ends. #1591

Closed

omarjackman force-pushed the update/olark-operator-availability-notice branch 5 times, most recently from b85f0ef to a18b52f Compare December 17, 2015 07:24

omarjackman 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 Dec 17, 2015

omarjackman force-pushed the update/olark-operator-availability-notice branch 2 times, most recently from f9ddf51 to 9bb9f1c Compare December 17, 2015 09:41

omarjackman changed the title ~~Help: Move operator availability notice~~ Help: Fix chat ended experience Dec 17, 2015

omarjackman added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 17, 2015

omarjackman force-pushed the update/olark-operator-availability-notice branch 2 times, most recently from 99ca2ff to 39629b4 Compare December 17, 2015 19:12

omarjackman 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 Dec 17, 2015

dmsnell reviewed Dec 17, 2015
View reviewed changes

dmsnell added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 17, 2015

Pass original olark event arguments to their listeners.

ec7544c

omarjackman force-pushed the update/olark-operator-availability-notice branch from 39629b4 to 43c8929 Compare December 17, 2015 19:58

omarjackman added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 17, 2015

Move the olark operator availability notice logic to the olark module…

0953de4

… and discontinue showing these notices in the olark widget as visitor notices. Also keep chat inlined if chat ends while on the contact form with no available operators.

omarjackman force-pushed the update/olark-operator-availability-notice branch from 43c8929 to 0953de4 Compare December 17, 2015 20:55

dmsnell added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 17, 2015

mtias reviewed Dec 17, 2015
View reviewed changes

omarjackman added a commit that referenced this pull request Dec 18, 2015

Merge pull request #1714 from Automattic/update/olark-operator-availa…

125d229

…bility-notice Help: Fix chat ended experience

omarjackman merged commit 125d229 into master Dec 18, 2015

scruffian removed the [Status] Ready to Merge label Dec 18, 2015

omarjackman deleted the update/olark-operator-availability-notice branch December 18, 2015 01:24

folletto mentioned this pull request Jan 4, 2016

Help: banners appearing at random times #2076

Closed

snyk-bot mentioned this pull request Aug 21, 2020

[Snyk] Security upgrade grunt from 1.0.3 to 1.3.0 awaisab172/wp-calypso#35

Open

snyk-bot mentioned this pull request Apr 22, 2021

[Snyk] Security upgrade grunt from 1.0.3 to 1.3.0 awaisab172/wp-calypso#69

Open

awaisab172 mentioned this pull request Nov 28, 2023

[Snyk] Fix for 17 vulnerabilities awaisab172/wp-calypso#169

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Help: Fix chat ended experience #1714

Help: Fix chat ended experience #1714

omarjackman commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dllh commented Dec 17, 2015

dllh commented Dec 17, 2015

omarjackman commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell Dec 17, 2015

dmsnell Dec 17, 2015

dmsnell commented Dec 17, 2015

dllh commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell commented Dec 17, 2015

omarjackman commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell commented Dec 17, 2015

mtias Dec 17, 2015

mtias Dec 17, 2015

Help: Fix chat ended experience #1714

Help: Fix chat ended experience #1714

Conversation

omarjackman commented Dec 17, 2015

Fix "chat has ended" experience.

Whats changed

How to test

omarjackman commented Dec 17, 2015

dllh commented Dec 17, 2015

dllh commented Dec 17, 2015

omarjackman commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell Dec 17, 2015

Choose a reason for hiding this comment

dmsnell Dec 17, 2015

Choose a reason for hiding this comment

dmsnell commented Dec 17, 2015

dllh commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell commented Dec 17, 2015

omarjackman commented Dec 17, 2015

omarjackman commented Dec 17, 2015

dmsnell commented Dec 17, 2015

mtias Dec 17, 2015

Choose a reason for hiding this comment

mtias Dec 17, 2015

Choose a reason for hiding this comment