Patch to fix Centering "Jumpiness" in KitchenSink

[Rajeev Dayal]

Hi Scott,

I'd like you to review this patch to fix some of the centering issues in KitchenSink. To apply this patch, perform the following steps:

svn update -r1178
patch -p0 < kitchensink-centering-fixes-r1178.patch

Description of the issue:

When clicking on the "Widgets" and "Lists" tabs, the content jumps from the left edge of the page to the center of the page. This is because we are showing the content, and then centering it. To get around this, the visibility of the content is set to hidden, the centering is done, and then the the visibility is set to visible. The same problem happens when you click on the "Show Dialog" button under the "Popups" section, and a similar fix is used to get around this problem.


Thanks,
Rajeev

[Joel Webber]

LGTM.

[Scott Blum]

LGTM.  Committed as r1181.

On 5/29/07, Rajeev Dayal <rda...@google.com> wrote:

[Adil]

I tried this patch and I don't think this solves the issue. I still
see the scroll bar flashing on and off in Firefox.

Shouldn't the setVisible(false) be before the show() like so:

dlg.setVisible(false);
dlg.show();
dlg.center();
dlg.setVisible(true);

or am I missing something?

Adil

[Rajeev Dayal]

Hi Adil,

On which tab are you seeing this problem? What version of Firefox are you using?

dlg.setVisible(false) is not before dlg.show() because in Internet Explorer, setVisible also sets the visibility of the IFRAME shim. The shim is not initialized until dlg.show() is called, so calling setVisible beforehand causes a NPE.

In this RC cycle, we are looking for a more elegant solution to the problem of needing to call show() before center().


Rajeev

[Adil]

Hello Rajeev,

I hate it when reality gets in the way like this. The scrollbar
flashes on the Popups tab when you click the 'Show Dialog' button. It
happens on the latest release Firefox (2.0.0.4) on my Powerbook G4
when the browser window is no more than 4/5 the height of the screen.

That gave me an idea and I may have a solution...

I think when the dialog pop-up is created the position is undefined
and somewhere to the bottom left of my window. So if I set its
position to 0,0 before the show() the problem does not happen. Like
so:

DialogBox dlg = new MyDialog();
dlg.setPopupPosition(0, 0);
dlg.show();
dlg.setVisible(false);
dlg.center();
dlg.setVisible(true);

Maybe the more elegant solution would be to put the setPopupPosition,
show and setVisible in the PopupPanel constructor and before any
contents are added to popup.

best

Adil

On Jun 4, 6:51 pm, "Rajeev Dayal" <rda...@google.com> wrote:
> Hi Adil,
>
> On which tab are you seeing this problem? What version of Firefox are you
> using?
>
> dlg.setVisible(false) is not before dlg.show() because in Internet Explorer,
> setVisible also sets the visibility of the IFRAME shim. The shim is not
> initialized until dlg.show() is called, so calling setVisible beforehand
> causes a NPE.
>
> In this RC cycle, we are looking for a more elegant solution to the problem
> of needing to call show() before center().
>
> Rajeev
>

[Rajeev Dayal]

Hi Adil,

Thanks for the information. I'm now seeing the same problem as you. Interestingly enough, the patch I submitted was not for the problem that you mention. You discovered a new problem :).

We should probably initialize the position of the popup to 0,0 in the constructor.

Instead of all of this skulduggery with setVisible, what we really need is the ability to call dlg.center() before dlg.show(). Right now, this is not possible, because the calculation of the the centered position in center() is dependent upon the Dialog being attached to the DOM. The call to show() attaches the Dialog to the DOM.

We need to figure out a way to correctly compute the centered position when the popup is not attached to the DOM. One way in which we could do this is store the offsetWidth and offsetHeight of the popup whenever hide() is called. However, the problem with this solution is that the child widget in the PopupPanel could change in some fashion in between the calls to hide() and center(), which would affect the offsetWidth and offsetHeight of the popup.

Another solution would be to add another method, centerAndShow(), which would basically do the same trick as we are doing in KitchenSink.

I think the best solution might be to have the call to center() actually call show() in the event that the the popup is not showing. Then, we could change the implementation of center() to use the same trick as we do in KitchenSink to avoid the "jumpiness" problem.


Rajeev

On 6/4/07, Adil < adil....@gmail.com > wrote:

Hello Rajeev,

I hate it when reality gets in the way like this. The scrollbar
flashes on the Popups tab when you click the 'Show Dialog' button. It

happens on the latest release Firefox ( 2.0.0.4) on my Powerbook G4