Merge lp:~huwshimi/launchpad/ajax-time into lp:launchpad
Proposed by
Huw Wilkins
Status: | Merged |
---|---|
Approved by: | Robert Collins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12699 |
Proposed branch: | lp:~huwshimi/launchpad/ajax-time |
Merge into: | lp:launchpad |
Diff against target: |
183 lines (+132/-4) 4 files modified
lib/canonical/launchpad/icing/style-3-0.css (+38/-2) lib/canonical/launchpad/templates/launchpad-loginstatus.pt (+7/-1) lib/lp/app/javascript/ajax_log.js (+85/-0) lib/lp/app/templates/base-layout.pt (+2/-1) |
To merge this branch: | bzr merge lp:~huwshimi/launchpad/ajax-time |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Collins (community) | Approve | ||
William Grant | code* | Approve | |
Review via email: mp+55257@code.launchpad.net |
Commit message
[r=lifeless,
Description of the change
We want to expose the time AJAX events take. This branch adds a visible log of all AJAX events to the user logout area. It exposes time taken, the internal AJAX event id, HTTP status and OOPS ID (if applicable).
The log also highlights if the AJAX event takes more than 1 second.
A screencast can be found here: http://
To post a comment you must log in.
Looks really nice, but a few comments:
- That JS should be somewhere other than the template. Probably in a new file in lib/lp/ app/javascript.
- The spacing around the new link is also a little off... it might be best to just remove the • from the render time.
- You construct HTML in JS with string concatenation. While the values that you're including should always be safe, I'd prefer if you constructed a node from a literal string and injected the text using YUI/DOM methods.
- Could the click handler use toggleClass instead of add/removeClass?