Faux LiveChat Target existing window if available

[getdave]

See paYJgx-1nM-p2#comment-1735

This allows the faux inline help to open in an existing window if it's available rather than a new tab each time. This is like how the Preview feature works in WordPress.org in that if you have an existing preview window open and you click Preview it will try to open in the existing window.

Changes proposed in this Pull Request:

• Attempt to open faux live chat link in existing window to avoid multiple duplicate tabs.

Jetpack product discussion

paYJgx-1nM-p2#comment-1735

Does this pull request change what data or activity we track or use?

No

Testing instructions:

• In local JP env go to WPAdmin.
• Click the ? icon (bottom right).
• See it open in new tab.
• Switch back to WPAdmin tab.
• Click the ? icon again.
• It should open in the existing tab you create on the first click rather than another duplicate tab.
• Check the Calypso page still tracks the event - DevTools -> Network -> search for t.gif and check request params.

Or to test on Simple simply check out the diff D57816-code and repeat steps above on your sandboxed Simple site.

Proposed changelog entry for your changes:

• Opens faux live chat link in the existing tab/window to avoid multiple duplicate tabs.

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello getdave! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D57816-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• 🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Include a changelog entry for any meaningful change.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

If you are an automattician, once your PR is ready for review add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.undefined

---

jetpack plugin:

• Next scheduled release: April 6, 2021.
• Scheduled code freeze: March 29, 2021

[mmtr]

Nice improvement, although I don't think this fully addresses the concern raised in paYJgx-1nM-p2#comment-1735 which was:

• While on WP Admin, user clicks on ? button.
• In the new tab, users clicks on a WP Admin link.
• User gets a new tab, but we should redirect them to the previous tab.

I see this is still happening (and honestly I don't see any way of avoiding it):

Also left some comments regarding the implementation.

[getdave]

I see this is still happening (and honestly I don't see any way of avoiding it):

I agree - i don't see how we solve that particular issue. I think I must have linked up the wrong thread though. I'm simply trying to avoid opening multiple tabs if the user goes back to Calypso and clicks ? which I think is a valid aim.

[getdave]

cc @mrfoxtalbot This PR should ensure that clicks on ? will re-open an existing tab rather than create a new one (assuming the tab is one originally created by clicking on this link).

[mmtr]

It seems that when there is an existing window, we only focus it? Shouldn't we also redirect to /help?

[getdave]

It seems that when there is an existing window, we only focus it? Shouldn't we also redirect to /help?

Ah that was a good catch. I fixed this to always reload the URL when focusing existing tab.

[mrfoxtalbot]

Thank you @getdave! I opened a separate issue in HC to make sure that the FAB (conversation icon/link) is visible there. 1661-gh-happychat

That being said, I am not sure now if this would make more harm than good. If a user is aware of the fact that they have two tabs they should not click on the FAUX FAB icon at all, but if they do they might get confused by seeing their tab reload.

How would this work if they had unsaved changes on the target tab? Would they be asked to save before reloading?

Thanks again!

[tjcafferkey]

Aside from the non-blocking comment I left this works well for me and feels like a much nicer experience 😁

[mmtr]

That being said, I am not sure now if this would make more harm than good. If a user is aware of the fact that they have two tabs they should not click on the FAUX FAB icon at all, but if they do they might get confused by seeing their tab reload.

This is a very valid concern, and something I didn't though about. I'm leaning towards leaving things as they are now, since it's the safest option. Yes, a user might end up with dozens of tabs open if they click on the ? dozens of times, but I think that's preferable over forcing a navigation on a tab that may result in undoing user actions done in that tab.

How would this work if they had unsaved changes on the target tab? Would they be asked to save before reloading?

It'd depend on the page the user is seeing on that tab. Some pages displays an unsaved changes warning, but others don't.

[getdave]

@mrfoxtalbot I'll leave this with you. If you feel it's an enhancement then we can land it otherwise feel free to close out.

[mrfoxtalbot]

I feel that I was coming from a very specific place (the risk of a user losing an ongoing chat) and missed the bigger picture of users who access chat via the Faux Fab in the first place, which is almost certainly going to be the most common scenario.

We might want to revisit this later on, once we have launched the Unified Navigation but I would leave it for now, in the spirit of
Primum non nocere.

Thank you all!

[getdave]

I would leave it for now

@mrfoxtalbot As in you'd prefer to Close and not merge this PR?

Primum non nocere.

You lost me here 😆

[mrfoxtalbot]

Sure, I am going to close this PR, we can revisit it later.

By Primum non nocere (First No Harm) I meant that it was not 100% clear if this was going to do more harm than good, so it was probably best to not make the change it for now.

Thanks again!