Closed Bug 1104927 Opened 5 years ago Closed 5 years ago

UITour: Add Loop conversation view target for email/copy link buttons

Categories

(Firefox :: General, defect)

defect
Not set
Points:
8

Tracking

()

RESOLVED FIXED
Firefox 37
Iteration:
37.2
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

Attachments

(1 file, 1 obsolete file)

We won't auto-open the conversation view since that is quite a bit of work so this depends on bug 1080943.

Things to handle:
* Update availableTargets to reflect whether a conversation window is open.
* Figure out what to do with multiple conversations. I propose we use the selected (non-minimized) one.
* Handle the conversation view popping out. I believe the tour always looks for targets in the same ChromeWindow at the moment so this may be tricky.

There is WIP code in attachment 8528515 [details] [diff] [review] for getting the selected conversation chatwindow.
Flags: qe-verify-
Flags: firefox-backlog+
hideLoopPanelAnnotations will need to be update to not auto-hide annotations from the conversation view when the loop panel closes. We should close the targets in the converation window when it is closed though.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 37.1
Iteration: 37.1 → 37.2
Attached file MozReview Request: bz://1104927/MattN (obsolete) —
Attachment #8534857 - Flags: review?(mixedpuppy)
Attachment #8534857 - Flags: review?(bmcbride)
/r/1411 - Bug 1104927 - UITour: Add Loop conversation view target for email/copy link buttons. r=Unfocused,mixedpuppy

Pull down this commit:

hg pull review -r e0d238caf1fdb43fcb92541a64c942498a6a9b78
/r/1411 - Bug 1104927 - UITour: Add Loop conversation view target for email/copy link buttons. r=Unfocused,mixedpuppy

Pull down this commit:

hg pull review -r 6c1a6e6e0134c8b11f0a4011885dd2dc63d11caf
Comment on attachment 8534857 [details]
MozReview Request: bz://1104927/MattN

Seems reasonable to me.
Attachment #8534857 - Flags: review?(mixedpuppy) → review+
agibson, in the interest of time/complexity I changed my mind and decided not to handle all of the ways that the annotation target could disappear with UITour and instead notify the webpage about the various events so it can call hideInfo(). Let me know if this won't work.

The event names for observe are listed at https://reviewboard.mozilla.org/r/1409/diff/#file18335 with the "Loop:" prefix.
https://reviewboard.mozilla.org/r/1409/#review853

::: browser/components/loop/MozLoopService.jsm
(Diff revision 2)
> -          UITour.notify("Loop:ChatWindowClosed");
> +          UITour.availableTargetsCache.clear();

I feel like we need a test for this, checking whether the conversation-related targets are in availableTargets or not.
I really want to make a test for this but I think it will take so much time that it's not worthwhile since we will have manual testing of the tours when we use these targets. The problem is that I think I have to mock a bunch of loop code, possibly even fake servers in order to open a room.
Ouch :( Ok, fair enough.
Attachment #8534857 - Flags: review?(bmcbride) → review+
(In reply to Matthew N. [:MattN] from comment #6)
> agibson, in the interest of time/complexity I changed my mind and decided
> not to handle all of the ways that the annotation target could disappear
> with UITour and instead notify the webpage about the various events so it
> can call hideInfo(). Let me know if this won't work.
> 
> The event names for observe are listed at
> https://reviewboard.mozilla.org/r/1409/diff/#file18335 with the "Loop:"
> prefix.

Thanks, Matt

Will report back if I hit any issues
https://hg.mozilla.org/mozilla-central/rev/6acf307a8620
https://hg.mozilla.org/mozilla-central/rev/8778e0d6bba5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
Attachment #8534857 - Attachment is obsolete: true
Attachment #8618723 - Flags: review+
You need to log in before you can comment on or make changes to this bug.