Closed Bug 972020 Opened 10 years ago Closed 10 years ago

Desktop client needs UI to minting "call-me" URLs

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla33
backlog mlp+

People

(Reporter: abr, Unassigned)

References

Details

(Whiteboard: [landed on loop-ui-initial][s=mlpnightly1, p=.25][qa+])

User Story

As a user, I can click on the Loop icon in the toolbar and get a URI to share with a friend.

Assumes any necessary authentication with the server is already completed.

Approximate UX:

- Panel shows some text "Get link to share with a friend to Video Chat" and a button.
- A text box is displayed for entering the person's name
- Clicking the button gets a link from the server
- The link is displayed in a read-only text box
- The button may be pressed more times to get more links

Attachments

(1 file)

      No description provided.
Blocks: 972015
No longer blocks: 972015
Blocks: 972014
Blocks: loop_mlp
No longer blocks: loop_mlp
Blocks: loop_mlp
Does this require signing into FxA first? (I can't remember if we said we needed to sign in for MLP or not)

What's the UX here?
User Story: (updated)
Flags: needinfo?(rtestard)
Flags: needinfo?(adam)
Blocks: 972024, 974875
No longer blocks: 972014, loop_mlp
No FxA signing for MLP was agreed.
Flags: needinfo?(rtestard)
Boriss can you please help with the UX?
Flags: needinfo?(boriss@mozilla.com)
(In reply to Mark Banner (:standard8) from comment #1)
> Does this require signing into FxA first? (I can't remember if we said we
> needed to sign in for MLP or not)

I think the goal is to have FxA in MLP if possible, but not to treat it as a hard blocker.

For MLP, the account aspect of creating URLs might not be critical, so long as you can map from the "call me" URL to the push URL in some way. For MVP, we need to ensure that we're minting "call me" URLs only for the user who requests them (that is, the request to create a "call me" URL needs to be authenticated, which implies FxA login). If we can get FxA into MLP, then it's probably best to do this correctly from the outset rather than cobbling together something that works without a login and then having to rework the code.
Flags: needinfo?(adam)
No longer blocks: 972024
Mark -- I'm assigning this to you as part of bug 974875.  Thanks.
Assignee: nobody → standard8
User Story: (updated)
As per my comment in the UX etherpad, if there's away to easily avoid forcing the user to click a button, that seems like nicer UX.  That said, if it's easier to do it with the button, that's clearly just a nice-to-have.
User Story: (updated)
Priority: -- → P1
Whiteboard: [est: 2d]
Assignee: standard8 → nperriault
Component: General → Client
We've implemented this on the github branch, it has some reliance on the chrome privs, so we may need some rework for this to work fully.
Whiteboard: [est: 2d] → [est: 2d][version implemented, paritially dependent on chrome, may need rework]
Depends on: 976109
Whiteboard: [est: 2d][version implemented, paritially dependent on chrome, may need rework] → [chrome based landed on loop-ui-initial, may need rework]
Whiteboard: [chrome based landed on loop-ui-initial, may need rework] → [landed on loop-ui-initial][may need rework]
backlog: --- → mlp+
Marking fixed as this has landed on loop-ui-initial and we believe the work here is done; loop-ui-initial will be cherry-picked to master at an appropriate time.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [landed on loop-ui-initial][may need rework] → [landed on loop-ui-initial]
Whiteboard: [landed on loop-ui-initial] → [landed on loop-ui-initial][s=mlpnightly1, p=.25]
This is a late review for the parts of this bug that landed on our integration branches a while ago, that we didn't review at the time due to not having our process fully sorted. We'll address comments where necessary in a follow-up patch, via bug 1005245.
Attachment #8422273 - Flags: review?(standard8)
Comment on attachment 8422273 [details] [review]
Link to Github pull-request: https://github.com/adamroach/gecko-dev/pull/23

There's a couple of minor comments that we'll deal with - one removing some unnecessary test code (we'll do that in bug 1005245), the other is sorting out our css and themes story, which I've filed bug 1010142 for.
Attachment #8422273 - Flags: review?(standard8) → review+
Landed on our current integration branch:

https://github.com/adamroach/gecko-dev/commit/804d9dcfacc8c3d7a57480f5bf03bd1ec15d462b

Will comment when this lands in mercurial.
Target Milestone: --- → mozilla33
Note for the next update: you need to add the MPL2 license header to the loop.properties
Please use meaningful string ids in the future.
(In reply to Stefan Plewako [:stef] from comment #15)
> Please use meaningful string ids in the future.

Correct bug? Because both strings and IDs seem clear to me.
(In reply to Francesco Lodolo [:flod] from comment #16)
> Correct bug? Because both strings and IDs seem clear to me.

Correct, https://hg.mozilla.org/mozilla-central/rev/76aa3d8bed6d
As I said, I can't see anything wrong with them. Can you explain exactly that the problem is, and how would you improve them?
Repeating string content in a string id is useless from localizer pov - there is no info or context passed what the string is about and without searching for screenshot in a bug or trying to find the string in the UI it is almost impossible to localize it correctly.
OK, makes more sense now. Use IDs related to the actual role of the string, not its content.
Marking this verified fixed based on previous smokestesting.
Status: RESOLVED → VERIFIED
QA Contact: anthony.s.hughes
Whiteboard: [landed on loop-ui-initial][s=mlpnightly1, p=.25] → [landed on loop-ui-initial][s=mlpnightly1, p=.25][qa+]
You need to log in before you can comment on or make changes to this bug.