Merge lp:~abentley/launchpad/js-usage into lp:launchpad

Proposed by Aaron Bentley
Status: Merged
Merged at revision: 12797
Proposed branch: lp:~abentley/launchpad/js-usage
Merge into: lp:launchpad
Prerequisite: lp:~abentley/launchpad/js-translation-2
Diff against target: 0 lines
To merge this branch: bzr merge lp:~abentley/launchpad/js-usage
Reviewer Review Type Date Requested Status
Deryck Hodge (community) code Approve
Review via email: mp+56655@code.launchpad.net

Commit message

Allow inline editing of translation settings.

Description of the change

= Summary =
Ajaxify setting translation usage.

== Proposed fix ==
Since the form used for updating translations is complicated, submit the form
to the appserver instead of using the web service.

== Pre-implementation notes ==
Discussed with deryck.

== Implementation details ==

Extracted update_form from replace_productseries.
Extracted form_url from replace_productseries.

Implemented usage_overlay.

Implemented submit_form to submit the form data from the usage overlay.

Implemented FormErrorHandler as a subclass of ErrorHandler to handle web page
form submission errors instead of web service errors.

Allow error handler to be specified for IOHandler.

Update tests to use the *visible* overlay, not the first overlay.

== Tests ==
bin/test --layer=WindmillLayer sharing_details
firefox lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html

== Demo and Q/A ==
Go to a +sharing-details page and click the edit icon next to "Translations
(are/are not) enabled on the upstream project."

A green checkmark should be shown when "Type of service for translations
application" is "External" or"Launchpad". The other values should be settable,
but have no impact on the main page.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/windmill/widgets.py
  lib/lp/app/javascript/lp.js
  lib/lp/translations/templates/sourcepackage-sharing-details.pt
  lib/lp/app/javascript/client.js
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
  lib/lp/app/javascript/picker.js
  lib/lp/translations/browser/tests/test_sharing_details.py
  lib/lp/translations/windmill/tests/test_sourcepackage_sharing_details.py
  lib/lp/translations/javascript/sourcepackage_sharing_details.js
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html
  lib/lp/translations/browser/sourcepackage.py

./lib/lp/testing/windmill/widgets.py
      29: E501 line too long (80 characters)
      35: E501 line too long (80 characters)
      36: E501 line too long (81 characters)
      95: E501 line too long (83 characters)
     122: E301 expected 1 blank line, found 0
      27: Line exceeds 78 characters.
      29: Line exceeds 78 characters.
      35: Line exceeds 78 characters.
      36: Line exceeds 78 characters.
      43: Line exceeds 78 characters.
      95: Line exceeds 78 characters.
./lib/lp/app/javascript/lp.js
      53: Expected '!==' and instead saw '!='.
     117: Expected '!==' and instead saw '!='.
     187: Expected '===' and instead saw '=='.
     290: Expected '===' and instead saw '=='.
     305: Expected '===' and instead saw '=='.
     306: Expected '===' and instead saw '=='.
     338: Move 'var' declarations to the top of the function.
     338: Stopping. (76% scanned).
       0: JSLINT had a fatal error.
./lib/lp/app/javascript/client.js
      30: Expected '===' and instead saw '=='.
      33: Expected '===' and instead saw '=='.
      54: Move 'var' declarations to the top of the function.
      54: Stopping. (5% scanned).
       0: JSLINT had a fatal error.
       4: Line exceeds 78 characters.
     191: Line exceeds 78 characters.
     570: Line exceeds 78 characters.
     618: Line exceeds 78 characters.
     745: Line exceeds 78 characters.
./lib/lp/app/javascript/picker.js
      43: Expected ';' and instead saw 'if'.
      92: Expected '!==' and instead saw '!='.
     223: Expected '!==' and instead saw '!='.
     224: Expected '{' and instead saw 'temp_spinner'.
     257: Expected '!==' and instead saw '!='.
     270: 'header' used out of scope.
     271: 'step_title' used out of scope.
     319: Move 'var' declarations to the top of the function.
     319: Stopping. (85% scanned).
       0: JSLINT had a fatal error.
     194: Line exceeds 78 characters.

To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

Looks good again, Aaron. Well done on all this JavaScript work! The inheritance looks good for FormErrorHandler, and the branch otherwise is nice, too. I'll mention the brackets and spaces issue again, just for consistency, but again mark this as approved.

Thanks so much for your hard work on this! I know you put a lot of time and effort into this. The work will be extremely useful to people on this page, and I hope the next time you come to a non-small JavaScript feature it will be easier due to the time spent here.

Cheers,
deryck

review: Approve (code)

Preview Diff

Empty