Skip to content
This repository has been archived by the owner. It is now read-only.

Prompt for profile image and display name on the permissions screen #2477

Closed
zaach opened this issue May 26, 2015 · 17 comments
Closed

Prompt for profile image and display name on the permissions screen #2477

zaach opened this issue May 26, 2015 · 17 comments
Assignees
Labels

Comments

@zaach
Copy link
Contributor

@zaach zaach commented May 26, 2015

We should expand the permissions screen to prompt for the user's profile image.

@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Jan 4, 2016

For Mozilla properties, FxA should be able to pass off email, display name and/or avatar. If any are not needed they do not appear. If any are required, it should be indicated.
request-for-permission

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 4, 2016

@rfk, if we add this feature then would Pocket get the same treatment as AMO?

Also I think we mignt need to enable the permission screen for TRUSTED reliers, because only untrusted get the permission screen right now...

cc @zaach

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 4, 2016

Also we might need an AMO milestone

@rfk
Copy link
Member

@rfk rfk commented Jan 4, 2016

if we add this feature then would Pocket get the same treatment as AMO?
[...]
we mignt need to enable the permission screen for TRUSTED reliers, because only
untrusted get the permission screen right now

Right; we'll have some work to do to enable this prompt for trusted Mozilla reliers, but that means we can isolate this change to AMO and keep Pocket the way it is.

@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Jan 5, 2016

For Mozilla properties, FxA should be able to pass off email, display name and/or avatar. If any are not needed they do not appear. If any are required, it should be indicated.

Let's deal with non-Mozilla properties on a case-by-case basis, but ideally we can move toward offering a UUID-based anomymous sign-in that doesn't hand over identity and is on-mission.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 11, 2016

@ryanfeeley to check the priority

@andymckay
Copy link

@andymckay andymckay commented Jan 11, 2016

We are hoping to turn it on in AMO in the next few weeks and although I think we can live without this functionality, it would sure be nice to have though.

@rfk rfk added this to the FxA-71: AMO login support milestone Jan 11, 2016
@rfk
Copy link
Member

@rfk rfk commented Jan 11, 2016

I'm not sure how much work the "enable the permission screen for TRUSTED reliers" part of this will be, if we don't want to enable it uniformly for all trusted reliers...

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jan 15, 2016

We are hoping to turn it on in AMO in the next few weeks and although I think we can live without this functionality, it would sure be nice to have though.

@andymckay - I was out for a while so might have missed discussions about this feature; the timetable comes as a bit of a surprise. Is there a concrete date? The absolute earliest we can get this into production is the week of Feb 1st, with all the standard caveats that come with making such a claim.

For a Feb 1st release we have to have strings "locked" by Jan 22nd for l10n string extraction, and the feature complete by Jan 25th for the train freeze. My gut feeling is the questions @rfk raised about surfacing this feature in a way that doesn't break other reliers, and getting this prioritized with other work may make adding this feature to the Feb 1st release unrealistic.

@andymckay
Copy link

@andymckay andymckay commented Jan 15, 2016

We've been chatting about AMO going live with FxA since Q4, but this new
functionality wasn't discussed until it was brought up in this bug. As I
said, I'm good to move forward without it, its not a blocker.

The current deployments in AMO are in flux as we undergoing emergency
stability fixes, once we feel AMO is stable again, we'll get back onto our
regular deployment schedule.

On Fri, Jan 15, 2016 at 3:21 AM, Shane Tomlinson notifications@github.com
wrote:

We are hoping to turn it on in AMO in the next few weeks and although I
think we can live without this functionality, it would sure be nice to have
though.

@andymckay https://github.com/andymckay - I was out for a while so
might have missed discussions about this feature; the timetable comes as a
bit of a surprise. Is there a concrete date? The absolute earliest we can
get this into production is the week of Feb 1st, with all the standard
caveats that come with making such a claim.

For a Feb 1st release we have to have strings "locked" by Jan 22nd for
l10n string extraction, and the feature complete by Jan 25th for the train
freeze. My gut feeling is the questions @rfk https://github.com/rfk
raised about surfacing this feature in a way that doesn't break other
reliers, and getting this prioritized with other work may make adding this
feature to the Feb 1st release unrealistic.


Reply to this email directly or view it on GitHub
#2477 (comment)
.

@shane-tomlinson shane-tomlinson self-assigned this Jan 19, 2016
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jan 19, 2016

@ryanfeeley - I am doing some initial front-end work for the UI. If the user has a profile image set, should it be displayed in the permissions screen next to "Account picture"?

@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Jan 19, 2016

@shane-tomlinson I had originally wanted that, but never managed to make it look right.  Facebook also never bothered to include it (although it's non-optional for them) so I feel better about that. Seeing as it looks as though only about 7% of registering users set an avatar, the important part is for that none of the checkboxes appear if there is no data (i.e. no display name, no avatar).
opera-superman

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 25, 2016

@rfk in my mind I am worried that this milestone is not defined in https://github.com/mozilla/fxa and we are losing scope of this. We also had a quick chat last week about how these permission checks will require server support / changes.

shane-tomlinson pushed a commit that referenced this issue Jan 25, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
@shane-tomlinson shane-tomlinson changed the title Prompt for profile image on the permissions screen Prompt for profile image and display name on the permissions screen Jan 25, 2016
@rfk
Copy link
Member

@rfk rfk commented Jan 26, 2016

I am worried that this milestone is not defined in https://github.com/mozilla/fxa

You mean as a feature card under https://github.com/mozilla/fxa/tree/master/features ?

@rfk
Copy link
Member

@rfk rfk commented Jan 26, 2016

Adding a note while I think of it...in the meeting we talked about using query-params to allow the relier to opt-in to this kin of prompt even if they're trusted, and I was thinking of prompt=consent as defined by the OIDC spec here: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

I think @vladikoff is right that this is growing too big to push through in this single issue, and we should take a step back and spec it out in a feature card.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jan 26, 2016

You mean as a feature card under https://github.com/mozilla/fxa/tree/master/features ?

Sorry, exactly!

shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 17, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 18, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 19, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 19, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 19, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 19, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 19, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 22, 2016
Shane Tomlinson
issue #2477
shane-tomlinson pushed a commit that referenced this issue Feb 23, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 23, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
shane-tomlinson pushed a commit that referenced this issue Feb 23, 2016
Add separate fields for "email", "display name" and "profile image"

fixes #2477
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants