Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

buttons that could be potentially slow should have a spinner #1093

Closed
skinny97214 opened this Issue Feb 10, 2012 · 4 comments

Comments

Projects
None yet
6 participants

Especially for mobile, we need to signal that the button press was recorded.

@ghost ghost assigned lloyd Mar 8, 2012

@ghost ghost assigned shane-tomlinson Apr 3, 2012

Contributor

sawyerh commented Jun 29, 2012

@shane-tomlinson - I've added styles and a loader.gif asset, but wasn't sure what buttons this should be applied to or which .js file to implement that functionality in. All that really needs done is to apply a loading class to the button

https://github.com/sawyerh/browserid/commit/e7a3f0145e33987ff4f6a6db3efc65c528965339

shane-tomlinson added a commit that referenced this issue Jul 2, 2012

Merge pull request #1879 from sawyerh/issue_1093_button_spinners
Fixes for issue #868 and issue #1517 and styles/assets for issue #1093

@sawyerh - great work on this.  For the future, it is *much* easier to review pull requests that focus on exactly one item.  This would have been easier to review had it been 3 PR's - one for the spinner, one for the iPad update, and one for the button styles - this is something I am working on myself - @lloyd calls me out on it frequently.

Once I updated the frontend unit tests to include the front-end test branch, all ran as expected.  I manually tested the dialog displayed correctly in the iPad sheet program @st3fan produced, Android 2.3.5 native, Fennec Android 2.3.5 mobile phone, Android 4.0 native, Fennec Nightly on Android 4.0, Chrome on Android 4.0, Opera Mobile on Android 4.0, iOS 5 iPod, and iOS5 iPad.

The loader gif still needs hooked up - but it looks great on our buttons.

close #868
close #1517
issue #1093

r+
Member

jrgm commented Jul 10, 2012

spinner looks good in train-2012.07.06 when applied with dev tools. Leaving this issue open to actually make use of it where appropriate.

@seanmonstar seanmonstar closed this Aug 7, 2012

Member

jrgm commented Aug 14, 2012

Okay, I've been using current dev with PR GH-2214 on multiple desktop browsers and I do see the spinner appear when clicking through buttons. I'll get to mobile later. Offhand, with a fast browser on a reasonable speed connection, it's a bit of a distraction, but I'll let other folks kick the tires, and see if they think the same.

Member

jrgm commented Aug 20, 2012

verified train-2012.08.17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment