Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(33)

Issue 12932044: Fixes AC expanding the header in fullscreen

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by j.c.sackett
Modified:
10 years, 8 months ago
Reviewers:
rharding, jeff.pihach, mp+180575
Visibility:
Public.

Description

Fixes AC expanding the header in fullscreen For some reason the AC widget was losing its position: absolute setting in IE10. YUI should just handle this. The solution is simply to ensure the search widget sets position: absolute on the AC widget at render. This also adds a test to ensure the setting is right; it passes in IE10, and this looks to work on all browsers. Please also QA it, both to ensure stuff isn't goofy on other browsers and that the expansion isn't occuring in IE10. https://code.launchpad.net/~jcsackett/juju-gui/ac-moves-the-screen-down/+merge/180575 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 3

Patch Set 2 : Fixes AC expanding the header in fullscreen #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -0 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/widgets/charm-search.js View 1 1 chunk +5 lines, -0 lines 0 comments Download
M test/test_browser_search_widget.js View 1 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 6
j.c.sackett
Please take a look.
10 years, 8 months ago (2013-08-16 14:13:10 UTC) #1
rharding
LGTM with comments noting how we got here please. Thanks! https://codereview.appspot.com/12932044/diff/1/app/widgets/charm-search.js File app/widgets/charm-search.js (right): https://codereview.appspot.com/12932044/diff/1/app/widgets/charm-search.js#newcode268 ...
10 years, 8 months ago (2013-08-16 14:17:04 UTC) #2
jeff.pihach
Code LGTM - will QA https://codereview.appspot.com/12932044/diff/1/app/widgets/charm-search.js File app/widgets/charm-search.js (right): https://codereview.appspot.com/12932044/diff/1/app/widgets/charm-search.js#newcode268 app/widgets/charm-search.js:268: // Ensure that the ...
10 years, 8 months ago (2013-08-16 14:17:48 UTC) #3
jeff.pihach
QA OK!
10 years, 8 months ago (2013-08-16 14:24:08 UTC) #4
j.c.sackett
I've added the requested comments. Thanks!
10 years, 8 months ago (2013-08-16 14:31:55 UTC) #5
j.c.sackett
10 years, 8 months ago (2013-08-16 15:08:12 UTC) #6
*** Submitted:

Fixes AC expanding the header in fullscreen

For some reason the AC widget was losing its position: absolute setting in IE10.
YUI should just handle this.

The solution is simply to ensure the search widget sets position: absolute on
the AC widget at render.

This also adds a test to ensure the setting is right; it passes in IE10, and
this looks to work on all browsers. Please also QA it, both to ensure stuff
isn't goofy on other browsers and that the expansion isn't occuring in IE10.

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/12932044
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b