Merge lp://staging/~gary/launchpad/bug750561 into lp://staging/launchpad
Status: | Merged |
---|---|
Approved by: | Gary Poster |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12800 |
Proposed branch: | lp://staging/~gary/launchpad/bug750561 |
Merge into: | lp://staging/launchpad |
Diff against target: |
300 lines (+102/-20) 4 files modified
lib/canonical/launchpad/icing/style.css (+11/-0) lib/lp/registry/javascript/structural-subscription.js (+22/-5) lib/lp/registry/javascript/tests/test_structural_subscription.html (+2/-5) lib/lp/registry/javascript/tests/test_structural_subscription.js (+67/-10) |
To merge this branch: | bzr merge lp://staging/~gary/launchpad/bug750561 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Henning Eggers (community) | Approve | ||
Review via email:
|
Commit message
[r=henninge][bug=750561] add a spinner for when you delete, edit, or add a structural subscription.
Description of the change
This branch adds a spinner when you delete, edit, or add a structural subscription.
It does this with some simple CSS changes. What I've done seems like a pretty good pattern to follow for how to do this, because it seems very simple: just change classes around to spinner while you are waiting for the work to be done.
I added a couple of JS tests for this. They should be relatively straightforward to read. I added the ability of our LPClient stub to halt execution of the callback so you can examine the intermediate state. In order for the test to pass I had to add a dependency for lp.app.errors to work, and then I had to add some code to clean up the overlay that lp.app.errors adds.
While working on the tests, which involved a fair amount of debugging, I became annoyed and distracted by five errors I was seeing in the Chrome error log (but not in the test results). I traced the problem down to the fact that we were reusing the link for every test, which caused the errors as unpleasant side effects. Therefore, I made our setup code set up the link as well. This seemed to work nicely.
I also needed to simulate a click, and I noticed that there were two approaches to simulating a click in our tests: "Y.Event.
This is an incremental branch. In order to completely fix this bug, I will need to also add a spinner for the "muting" code in db-devel. I will do that next.
For QA, you should see the "remove" icon turn into a spinner when you click "Unsubscribe" on [structural subscription target]
Thank you
Gary
Thank you very much. I found this very interesting to read. ;-)