Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Split out identity verification from bank account connecting #3250

Merged
merged 14 commits into from
Mar 18, 2015

Conversation

Changaco
Copy link
Contributor

Closes #987.

@Changaco
Copy link
Contributor Author

Tagging TeamX ★ because I have other branches based on this.

@Changaco
Copy link
Contributor Author

Screenshot of invalid form field:
987

@chadwhitacre
Copy link
Contributor

On a fresh database, when I login and navigate to /about/me/identity, I get this traceback:

Traceback (most recent call last):
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
    new_state = function(**deps.as_kwargs)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 104, in get_response_for_resource
    return {'response': resource.respond(state)}
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/resources/simplate.py", line 66, in respond
    response = self.get_response(context)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/resources/simplate.py", line 185, in get_response
    response.body = render(context)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/renderers/__init__.py", line 107, in __call__
    return self.render_content(context)
  File "/Users/whit537/personal/gratipay/gratipay.com/gratipay/renderers/jinja2_htmlescaped.py", line 15, in render_content
    return base.Renderer.render_content(self, context)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen_jinja2_renderer.py", line 66, in render_content
    return self.compiled.render(context).encode(charset)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/jinja2/environment.py", line 969, in render
    return self.environment.handle_exception(exc_info, True)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/jinja2/environment.py", line 742, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/whit537/personal/gratipay/gratipay.com/www/%username/identity.spt", line 1, in top-level template code
    import balanced
  File "/Users/whit537/personal/gratipay/gratipay.com/templates/base.html", line 88, in top-level template code
    {% block content %}{% endblock %}
  File "/Users/whit537/personal/gratipay/gratipay.com/www/%username/identity.spt", line 31, in block "content"
    except ValueError:
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/jinja2/environment.py", line 397, in getattr
    return getattr(obj, attribute)
UndefinedError: 'balanced.resources.Customer object' has no attribute 'address'

@chadwhitacre
Copy link
Contributor

Identity should be included in the navigation. Right now the only way to get to it is through the bank account page.

@Changaco
Copy link
Contributor Author

Identity should be included in the navigation. Right now the only way to get to it is through the bank account page.

I think fixing Gratipay's navigation needs its own PR.

@Changaco
Copy link
Contributor Author

On a fresh database, when I login and navigate to /about/me/identity, I get this traceback:

The real problem here is that the browsing tests didn't detect that, I figured out why and am working on a PR to fix them.

@Changaco
Copy link
Contributor Author

UndefinedError fixed.

@chadwhitacre
Copy link
Contributor

I think fixing Gratipay's navigation needs its own PR.

#3258

@chadwhitacre
Copy link
Contributor

Needs a rebase?

- don't float, use `inline-block` for `.half`, prune the `clear` <div>s
- drop the unnecessary <div> with the `constrain-width clearfix` classes
- wrap the <label>s around the <input>s instead of using the `for` attribute
- fix the indentation of the info at the end of the page
- fix and complete the i18n of placeholders
- prune styles for unused classes (`.disabled`, `.nav`, `.full`)
- i18n of messages
- highlight the invalid fields and show the error message underneath it when focused
- autofocus the first invalid field in the form
- use notifications when we don't know which field to attach the error to
@chadwhitacre
Copy link
Contributor

UndefinedError fixed.

Confirmed.

@chadwhitacre
Copy link
Contributor

Tests passing locally. Reviewing UI ...

@chadwhitacre
Copy link
Contributor

Are these tooltips new on this PR? Definitely a move in the right direction.

screen shot 2015-03-17 at 12 04 55 pm

@chadwhitacre
Copy link
Contributor

Interesting that the tooltips I'm seeing on the Identity form are different than the ones screencapped from the credit card form at #3250 (comment).

@chadwhitacre
Copy link
Contributor

Regardless, I can save my bank account details now:

screen shot 2015-03-17 at 12 22 47 pm

@chadwhitacre
Copy link
Contributor

I don't see a way to remove my identity verification.

@chadwhitacre
Copy link
Contributor

I think we should add this to the profile nav, between Widgets and Settings.

@chadwhitacre
Copy link
Contributor

Next steps for me are to inspect what happens in Balanced when we post identity, and then read commits.

@chadwhitacre
Copy link
Contributor

  • 0360b58 clean up imports in www/{bank-account,credit-card}.html.spt
  • ef68732 split out identity verification from bank account connecting
  • 6545ff4 really disable the button instead of only changing its opacity
  • 0ef26ec i18n of delete confirmation message
  • 902c231 give the forms more accurate IDs
  • 0227177 clean up markup and style of ba-cc forms
  • 6616ab3 let the browser take care of required fields
  • d8981cc inline ba and cc data in the s like we do for the rest
  • e18bb31 improve error reporting
  • c305d79 fix UndefinedError
  • 18fb381 show a simple message when the identity is verified

@chadwhitacre
Copy link
Contributor

@chadwhitacre
Copy link
Contributor

It's goofy to wrap the "You need to verify your identity first" text (which, btw, should have a period) when we're not wrapping the long intro paragraph.

screen shot 2015-03-17 at 2 58 03 pm

@chadwhitacre
Copy link
Contributor

So ... the inline error reporting we've added here is good for pattern mismatches such as DOB, but it's bad for signaling required fields, because, whereas a pattern can be matched as soon as it's entered, we can't know that a required field is missing until the moment the form is submitted. But the moment the form is submitted, we can and should say when all required fields are missing, not just the first.

Firefox has okay behavior here. They highlight the missing fields, with a tooltip on the first:

screen shot 2015-03-17 at 3 17 18 pm

Chrome, however, doesn't highlight the missing fields:

screen shot 2015-03-17 at 3 18 55 pm

Are we doing something in CSS that prevents Chrome from highlighting missing fields?

@chadwhitacre
Copy link
Contributor

Required validation aside, it's shoddy to have some validation styled one way and some a different way.

Can we use browser validation for all fields?

@chadwhitacre
Copy link
Contributor

is it really so important to have all the tooltips look exactly the same?

Yes. Having some tooltips styled one way and others a different way is shoddy. Yes, we are quite shoddy in many ways. Let's not make our product worse.

@chadwhitacre
Copy link
Contributor

@Changaco We can punt on further error reporting clean-ups if you want to, since the main purpose of this PR is to separate the identity form from the bank account form.

@chadwhitacre
Copy link
Contributor

is it really so important to have all the tooltips look exactly the same?

Yes. Having some tooltips styled one way and others a different way is shoddy.

In fact, this is a perfect example of the "wagging the dog" antipattern that we've talked about before. To compete with closed companies, we "have to be as good or better on [...] features and UX" (IRC). We're not going to get there unless we really, truly own the difference between the product dog and the technology tail.

Wrong: "Hey, I've got this technology available. Where can I use it?"

Right: "Here's the experience I want my users to have. How do I deliver it?"

@chadwhitacre
Copy link
Contributor

product-dog

@Changaco
Copy link
Contributor Author

Yes. Having some tooltips styled one way and others a different way is shoddy.

I really don't get that. Would you have noticed the difference if I hadn't posted a screenshot of the red tooltip? Do you think users really care that one tooltip is white/gray and the other red? I can make ours gray if you want.

Yes, we are quite shoddy in many ways. Let's not make our product worse.

Compared to what we have now, this PR is still a big improvement even if the tooltips aren't of the same color.

@Changaco
Copy link
Contributor Author

Is that better?

3250

@chadwhitacre
Copy link
Contributor

Would you have noticed the difference if I hadn't posted a screenshot of the red tooltip?

Yes.

Do you think users really care that one tooltip is white/gray and the other red?

Yes. Any competent product designer (by definition, so say I) will consciously care that the tooltips are not consistent, because it's a distinction without a difference. The distinction in the product's interface does not encode a difference in the product's mental model. Rather, it encodes a tooling limitation. The tooling limitation has no meaning to the user. The mental model of the product is where the user looks for meaning.

Most Many untrained users will subconsciously care:

Can you guess the most frequently cited factor for evaluating the credibility of a Web site?

According to a 2002 study out of Stanford University, it is the “appeal of the overall visual design of a site, including layout, typography, font size, and color schemes,” (Fogg, et al., 2002). The look and feel of a site influenced judgments about credibility far more than other factors like structure, usefulness of the information, tone of the content, and name recognition!

04fig06

A different study found that “Web users form first impressions of Web pages in as little as 50 milliseconds (1/20th of a second).” What’s more, these initial attractiveness evaluations based on just a brief exposure “were very highly correlated with attractiveness evaluations of the same pages under unlimited exposure.*”

The Stanford study contributes to these guidelines, along with a number of other papers from the same era (1999-2002). Unfortunately, the links to the actual papers are broken. Admittedly, it's also old research, as noted:

The Persuasive Tech Lab did early research on the factors that affect the credibility of websites. Since our investigations in the 1990s, we’ve found that online credibility has morphed. With the rise of Web 2.0 services, the focus of credibility evaluations extend beyond the page to the people represented. In other words, Credibiltity [sic, ironically] 2.0 has become more like reputation, or perceived reputation.

@chadwhitacre
Copy link
Contributor

I really don't get that.

Hrm. That's a bigger problem, since historically I haven't had much luck getting you to get things you don't get. ;-)

I suppose the best I can do for now is to flag problems as I notice them, explain myself as best I can, and correct problems myself as much as I'm able. Hopefully over time we're able to attract and retain competent product designers to balance out our team.

@chadwhitacre
Copy link
Contributor

Compared to what we have now, this PR is still a big improvement even if the tooltips aren't of the same color.

Yeah, and what's more, the changes to form validation aren't the point of this PR. Splitting Identity out from the bank account page is the point. I'm almost ready to merge, lemme go over it one more time ...

@chadwhitacre
Copy link
Contributor

Are we doing something in CSS that prevents Chrome from highlighting missing fields?

P.S. I am able to provoke Chrome to highlight all required fields, though I haven't yet figured out how to do so consistently.

screen shot 2015-03-18 at 3 13 09 pm

@chadwhitacre
Copy link
Contributor

Is that better?

Somewhat. I'm not going to worry about it further here, because honestly we need to look at our forms overall (just as we need to look at our tables/listings overall; #3222). Reticketed as #3267 and noted on #3220.

@chadwhitacre chadwhitacre mentioned this pull request Mar 18, 2015
chadwhitacre added a commit that referenced this pull request Mar 18, 2015
Split out identity verification from bank account connecting
@chadwhitacre chadwhitacre merged commit a86e08e into master Mar 18, 2015
@chadwhitacre chadwhitacre deleted the identity-check-split branch March 18, 2015 19:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

split out identity verification from bank account connecting
2 participants