Skip to content

Conversation

mstriemer
Copy link
Contributor

@mstriemer mstriemer commented Jun 7, 2016

I had a little animation on the overlay at first but getting it to animate out was tricky so I removed it. It appears quite abruptly so we should probably add something but I'll let @pwalm say what. Maybe worth noting that I barely missed the close button there and it didn't register my click. I guess we should make that clickier.

error-overlay-nicer mov

No animation

error-overlay-animate-in mov

In animation

Fixes mozilla/addons#9611.

export function getFakeI18nInst() {
return {
gettext: sinon.stub(),
gettext: sinon.spy((str) => str),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, nice.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e78514a on mstriemer:error-reporting-again-395 into ca1f1c9 on mozilla:master.

@muffinresearch
Copy link
Contributor

r+wc

@mstriemer
Copy link
Contributor Author

I put 5px of padding on the close button so it's a bit easier to click. Still no animation on the overlay appearing, will wait for feedback from @pwalm first.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling be33211 on mstriemer:error-reporting-again-395 into ca1f1c9 on mozilla:master.

@mstriemer mstriemer merged commit c81d069 into mozilla:master Jun 7, 2016
@mstriemer mstriemer deleted the error-reporting-again-395 branch June 7, 2016 17:49
@pwalm
Copy link
Contributor

pwalm commented Jun 8, 2016

BRING ON THE ANIMATION! That looks sweet. But how about when the user turns on the switch, it turns back off (due to whatever error) and then the error animates in. That way the user will see that the switch didn't work before the error pops up.

@pwalm
Copy link
Contributor

pwalm commented Jun 8, 2016

Oh and good call on the 5px of padding 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hook up error handling
4 participants