refactor(permissions): change the style and copy of the permissions screen #2442
Conversation
|
Should "info" be "information" ? ("info" is a bit _info_rmal") |
|
The email input text colour, seems to be different from the design: Might need to make the text bit more light grey so the users understand that they cannot edit the input. |
| @@ -22,6 +22,7 @@ function () { | |||
| // it's already being translated and may be used in the future. | |||
| t('By proceeding, I agree to the <a id="service-tos" href="%(termsUri)s">Terms of Service</a> and' + | |||
| '<a id="service-pp" href="%(privacyUri)s">Privacy Notice</a> of %(serviceName)s (%(serviceUri)s).'); | |||
| t('Proceed'); | |||
vladikoff
May 20, 2015
Contributor
Do we need to Proceed here or have other strings?
We merged proceed here:
Do we need to Proceed here or have other strings?
We merged proceed here:
zaach
May 20, 2015
Author
Contributor
Ah, wanted to make sure 👍
Ah, wanted to make sure
|
Fixed styles, etc. Still need input on "info" and whether we want to link the relier name... seems like that could lead users astray/disrupt the flow. |
|
|
||
| <form id="permissions" novalidate> | ||
| <div class="input-row"> | ||
| <label class="label-helper visible">{{#t}}Email{{/t}}</label> |
shane-tomlinson
May 20, 2015
Member
label-helper is meant to enable placeholders to slide up when an input element is focused. Since the input element is disabled, that doesn't really make sense.
Instead of overloading label-helper's usage for styling, what about just removing the classes and updating the label's style directly.
label-helper is meant to enable placeholders to slide up when an input element is focused. Since the input element is disabled, that doesn't really make sense.
Instead of overloading label-helper's usage for styling, what about just removing the classes and updating the label's style directly.
zaach
May 20, 2015
Author
Contributor
Hm, yeah. We should probably separate classes for behaviors and for styles, so we can easily share styles even when behavior differs.
Hm, yeah. We should probably separate classes for behaviors and for styles, so we can easily share styles even when behavior differs.
| @@ -122,10 +122,10 @@ function (chai, $, sinon, p, View, AuthErrors, Metrics, FxaClient, | |||
| it('renders relier info', function () { | |||
| return initView('sign_up') | |||
| .then(function () { | |||
| assert(!! view.$('#fxa-permissions-header').text().match(SERVICE_NAME), | |||
| 'service name shows in header'); | |||
| assert(!! view.$('p').text().match(SERVICE_NAME), | |||
shane-tomlinson
May 20, 2015
Member
using p as a selector is pretty brittle to random passers-by like me who may try to add more to the template some day. Maybe use a class name.
using p as a selector is pretty brittle to random passers-by like me who may try to add more to the template some day. Maybe use a class name.
shane-tomlinson
May 20, 2015
Member
Another thing - you can use assert.notInclude here - http://chaijs.com/api/assert/#notInclude
Another thing - you can use assert.notInclude here - http://chaijs.com/api/assert/#notInclude
|
How about:
|
|
@zaach https://github.com/mozilla/fxa-content-server/blob/issue-2423-update-permissions/app/scripts/views/permissions.js#L58 submit: function () {
var self = this;
var account = self.getAccount();
self.logScreenEvent('proceed'); // <---------------- Should that say 'accepted'
return p().then(function () {
account.saveGrantedPermissions(self.relier.get('clientId'), self.relier.get('permissions'));
self.user.setAccount(account); |
|
Latest: |
refactor(permissions): change the style and copy of the permissions screen
@ryanfeeley How's this:
@vladikoff r?
Fixes #2423.