Closed Bug 1100382 Opened 5 years ago Closed 5 years ago

backport upstream bug 1090727 to bmo/4.2 to support jquery in the global/header template, and update the header and footer to use jquery

Categories

(bugzilla.mozilla.org :: User Interface, enhancement)

Production
enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dkl, Assigned: dkl)

References

Details

Attachments

(1 file, 3 obsolete files)

+++ This bug was initially created as a clone of Bug #1090727 +++

the global/header template needs updating to support loading jquery.

rough notes:
- load both jquery and yui2 on every page by default
- new header param "jqlibs" (or "jslibs"?)
  - initially support for jquery-ui only (loads smoothness css + js)
- new header param "noyui"
  - so pages can opt out of loading yui2
  - at some stage we need to invert this, so pages need to opt into yui2
- convert header, footer, and all dependent templates/js to jquery
  - required in order for pages to opt-out
  - yui hacks, such as onpagehide, can be removed if "noyui" is set
  - to simplify testing and reviewing avoid refactoring of code, a straight port is most useful
Attached patch 1100382_1.patch (obsolete) — Splinter Review
Attachment #8523907 - Flags: review?(glob)
Comment on attachment 8523907 [details] [diff] [review]
1100382_1.patch

Review of attachment 8523907 [details] [diff] [review]:
-----------------------------------------------------------------

this doesn't work with no_yui=1 because the BMO extension's usermenu uses YUI, resulting in a js error when no_yui=1.

for now it's ok to just disable the feature on no_yui=1 pages.  i have to create a jquery-ui version for my show_bug prototypes, and i'll create a separate bug to migrate the usermenu to jquery once that's done.
Attachment #8523907 - Flags: review?(glob) → review-
Attached patch 1100382_2.patch (obsolete) — Splinter Review
Attachment #8523907 - Attachment is obsolete: true
Attachment #8526140 - Flags: review?(glob)
Attached patch 1100382_3.patch (obsolete) — Splinter Review
+++ b/template/en/default/global/header.html.tmpl

+[% style_urls.import(jquery_css, jq_css_urls) FILTER null %]
Attachment #8526140 - Attachment is obsolete: true
Attachment #8526140 - Flags: review?(glob)
Attachment #8526216 - Flags: review?(glob)
Comment on attachment 8526216 [details] [diff] [review]
1100382_3.patch

Review of attachment 8526216 [details] [diff] [review]:
-----------------------------------------------------------------

this looks good overall; the context menu plugin you found looks nice :)

looks like t/008filter.t doesn't know about the 'null' filter..

t/008filter.t ........ 454/628
#   Failed test '(en/default) template/en/default/global/header.html.tmpl has unfiltered directives:
#   100: style_urls.import(jquery_css, jq_css_urls) FILTER null
# --ERROR'
#   at t/008filter.t line 130.

::: extensions/BMO/web/js/edituser_menu.js
@@ +36,5 @@
> +    selector: ".vcard_" + id,
> +    trigger: "left",
> +    items: items
> +  });
> +  // return false;

nit: remove commented out line

::: js/jquery/plugins/contextMenu/contextMenu.css
@@ +27,5 @@
> +        -ms-box-shadow: 0 2px 5px rgba(0, 0, 0, 0.5);
> +         -o-box-shadow: 0 2px 5px rgba(0, 0, 0, 0.5);
> +            box-shadow: 0 2px 5px rgba(0, 0, 0, 0.5);
> +    font-family: Verdana, Arial, Helvetica, sans-serif;
> +    font-size: 11px;

verdana is ugly; remove the font-family and size definitions so it uses the current skin's
Attachment #8526216 - Flags: review?(glob) → review-
Attached patch 1100382_4.patchSplinter Review
Attachment #8526216 - Attachment is obsolete: true
Attachment #8552499 - Flags: review?(glob)
Comment on attachment 8552499 [details] [diff] [review]
1100382_4.patch

Review of attachment 8552499 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob
Attachment #8552499 - Flags: review?(glob) → review+
Thanks!

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   a496214..5cc88fd  master -> master
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Depends on: 1129933
You need to log in before you can comment on or make changes to this bug.