launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

The proposal to merge lp:~jcsackett/launchpad/simplify-everything into lp:launchpad has been updated.

Description changed to:

Summary
=======
The previous work to make the banners work without js involved using an overly
complicated renderUI sequence that would replace existing HTML with the new
banner, but would have multiple banner objects. This branch massively
simplifies things by removing that bit of goofiness and using YUI's widget
srcNode attribute to safely reuse the html.

UPDATE: This also fixes a regression introduced by earlier banner work; after
the first banner.js branch landed +filebug was special cased in such a way that
it failed to work properly with private-by-default projects. Since this removes
the special casing, it has restored the functionality.

Preimp
======
Continuation of earlier work discussed with Curtis Hovey and Rick Harding.

Implementation
==============
The _createBanner and _updateBanner methods are discarded. _createBanner is
merged with renderUI; _updateBanner becomes updateText, since that's really
all it does.

The banner_id attribute and its uses are removed, most notably in the template
banner.

The beta banner is switched to using the same singleton banner as the privacy
banner, since it too exists as only one instance per page.

Both the beta banner and privacy banner are updated to get srcNode within
their wrapper functions. This can be null, b/c Y.Widget handles null srcNodes,
which just indicates that there's no html to update.

The privacy banner wrapper has been updated to use updateText to set new
banner texts instead of the config on the initializer. All calls to the
PrivacyBanner now use the wrapper.

test_only_one_banner is deleted, b/c it's not needed.

test_updateText is added.

test_privacy gets a test_only_one banner, which uses the wrapper function to
verify that only banner is created.

The html in banner-macros is updated to correspond exactly to the html
generated by the widgets, less the dynamically generated IDs from YUI.

Tests
=====
bin/test -vvc -t beta -t banner -t privacy -t type_choice --layer=YUI

QA
==
Confirm that no-js and js use of privacy and beta banners doesn't change.

Confirm that the banner shows up on +filebug for private-by-default projects.

LoC
===
This is part of disclosure work.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/banners/beta-notification.js
  lib/lp/app/javascript/banners/tests/test_banner.js
  lib/lp/app/javascript/banners/banner.js
  lib/lp/bugs/templates/bugtarget-macros-filebug.pt
  lib/lp/app/templates/banner-macros.pt
  lib/lp/app/javascript/banners/privacy.js
  lib/lp/app/javascript/banners/tests/test_privacy.js

./lib/lp/app/templates/banner-macros.pt
      41: mismatched tag

I believe this lint error is wrong; line 41 is a closing div that looks
matched. The reviewr is invited to tell me I'm wrong if they can see the
issue, in which case I'll fix it. I just don't see it, and macro compilation
still works.

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/simplify-everything/+merge/107240
-- 
https://code.launchpad.net/~jcsackett/launchpad/simplify-everything/+merge/107240
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.