-
Notifications
You must be signed in to change notification settings - Fork 308
Conversation
2f17cfc
to
024624d
Compare
Sentry has an awesome blur effect on the background when the modal is up. Don't have a screenshot handy but will keep an eye out ... |
We should also open the modal automatically on a 401 page... |
cdae08a
to
042aa11
Compare
ccbeff0
to
a0cb411
Compare
042aa11
to
7327ba6
Compare
a0cb411
to
43ac69f
Compare
7327ba6
to
a559476
Compare
Done in 985296e |
Going to deploy to staging.. |
Deployed to staging. Actual signing in/up won't work because we haven't configured Github properly, but the modal should work. 401 page: http://gratipay-staging.herokuapp.com/about/me/emails |
Works for me and passes tests locally. |
Why do we want this again? |
@mattbk This was extracted out of #1052. #1052 (comment) for reference |
(Added that to the PR description too) |
Gotcha. I thought I remembered something like that. |
!m @rohitpaulk So I think I am going to adopt a really low-bar code review strategy for the next month or two while I'm traveling, in order to not slow down our shipping. Basically, I'm going to skim looking for dangerous (i.e., security-related) blunders. Beyond that I'm not going to worry much about architecture, etc. You okay with that @rohitpaulk et al.? @mattbk Will you have bandwidth this summer to handle functional review? |
Probably. |
@mattbk Awesome, in that case let me know when this is good from a UX/functional point of view and I will skim the code again and merge. :-) |
It works for me and looks fine. |
43ac69f
to
c45e1b9
Compare
985296e
to
6990a05
Compare
Rebased, was 985296e. |
@whit537 Have you had a look at this yet? |
@rohitpaulk Yes! Just ran out of time on the road the other day. |
!m @whit537 I'm turning the staging app off. |
Part of #1052
extract-modal
tomaster
Screenshots: