Popup sample regression in popup panel size.

[AlecR]

I've just noticed a regression in this sample between 2.8.12 and 2.9.4. (Untested for earlier 2.9 versions.)

In 2.9.4, the ScrolledPopup does not display scrollbars; whilst in
2.8.12 it does for me. (The scroll and vscroll samples work for me in
2.9.4.)

I suspect that the regression is probably due to a sizer change, or perhaps the sample's code should be changed for 2.9.

[vadz]

It would be great if somebody could look at this, I don't know when exactly did this break but it looks like a bad enough problem to merit being fixed before 3.0.

[ericj]

I took a look at this, i think it wasn't broken, it was optimized.

wxScrollHelper::DoAdjustScrollbar() has code that hides the scrollbar if the client size is big enough for the whole content.

Even if you look at the old 2.8.x sample, you'll notice that the scroll area was pretty small, although the scrollbars were set to (1,1, 1000, 1000). Because the scrolled window is under control of a sizer, the sizer will re-set the virual size.

If you add " m_panel->SetAutoLayout(false); " to the end of the SimpleTransientPopup ctor, the scrollbars will be visible.

[AlecR]

Thanks for looking at this, and your helpful analysis. Of course the end result is that there is currently no visible difference between the effects of pressing the two buttons; yet the user might reasonably expect to see a difference (as was shown in 2.8x). Since this is in a sample, I personally think that it would be preferable not to leave the situation as it is. Do you not agree? (I'd agree, tho', that this is a very low priority issue.)

[vadz]

Yes, the thing that bothers me is that I don't see any relevant changes in the sample compared to 2.8. So why does it behave differently? I don't believe it's just a valid optimization neither because even with this:

samples/popup/popup.cpp

@@ -403 +403 @@
     wxLogMessage( wxT("================================================") );
     delete m_scrolledPopup;
     m_scrolledPopup = new SimpleTransientPopup( this );
-    m_scrolledPopup->GetChild()->SetScrollbars(1, 1, 1000, 1000);
+    m_scrolledPopup->GetChild()->SetScrollRate(10, 10);
+    m_scrolledPopup->GetChild()->SetVirtualSize(1000, 1000);
     wxWindow *btn = (wxWindow*) event.GetEventObject();
     wxPoint pos = btn->ClientToScreen( wxPoint(0,0) );
     wxSize sz = btn->GetSize();

the scrollbars are still not shown. So something is definitely wrong here, I just don't see what...

[ericj]

Calls to SetScrollRate(), SetVirtualSize() won't change anything as long as the window is in "autolayout" mode.

The following code in scrlwing.cpp will reset the virtual size:

void wxScrollHelperBase::HandleOnSize(wxSizeEvent& WXUNUSED(event))
{
    if ( m_targetWindow->GetAutoLayout() )
    {
        wxSize size = m_targetWindow->GetBestVirtualSize();

        // This will call ::Layout() and ::AdjustScrollbars()
        m_win->SetVirtualSize( size );
    }
    else
    {
        AdjustScrollbars();
    }
}

Adding " m_panel->SetAutoLayout(false); " to the end of the SimpleTransientPopup ctor disables autolayout and the scrollbars appear.

Like said before, the sample didn't even work correctly in 2.8.x, but as "some" scrollbars appeared, it probably wasn't noticed until now. The only difference is that 2.9.x now hides the scrollbars if they aren't needed.

Whether it's considered a bug that the sizer overrides the virtual size the client requested, is a different issue.

[vadz]

Replying to ericj:

Calls to SetScrollRate(), SetVirtualSize() won't change anything as long as the window is in "autolayout" mode.

Ah, yes, of course, sorry for missing this the first time you explained it.

Like said before, the sample didn't even work correctly in 2.8.x, but as "some" scrollbars appeared, it probably wasn't noticed until now. The only difference is that 2.9.x now hides the scrollbars if they aren't needed.

I'd still like to understand what exactly is the change here but it's likely to be correct/beneficial so the main motivation to find it would be to document it in docs/changes.txt. For now I'll just change the sample to show scrollbars.

Whether it's considered a bug that the sizer overrides the virtual size the client requested, is a different issue.

I think it makes sense for it to work like this, if you're using sizers you shouldn't set the size manually. But it might need to be documented too...

Thanks again for the explanations!

[ericj]

Replying to vadz:

I'd still like to understand what exactly is the change here but it's likely to be correct/beneficial so the main motivation to find it would be to document it in docs/changes.txt.

The magic happens in wxScrollHelper::AdjustScrollbars() in scrlwing.cpp. Apparently that method was refactored several times. Now, in 2.9.x it calls a new method wxScrollHelper::DoAdjustScrollbar() which hides the scrollbar if the client size is big enough to fit the virtual size. That code did not exist in 2.8.x.

[vadz]

Thanks for your debugging once again!

AFAIR this change wasn't intentional... But OTOH it does seem to make sense. Still strange that it didn't behave like this in 2.8 though, I'm pretty sure that scrolled windows didn't show scrollbars there if they were not needed in general. So could it be that this only didn't happen when they were managed by sizers?

[VZ]

(In [73319]) Ensure that scrollbars are shown in scrolled popup in the popup sample.

Don't call SetScrollbars(), this is overridden by sizer logic later. Instead,
add a sufficiently big window to the popup and make fix the size of the popup
itself to be smaller to ensure that the scrollbars do show.

Closes #14900.