ppescher / resizablelib

A set of MFC classes to easily make resizable windows
Other
55 stars 27 forks source link

Bug resizing a dialog smaller than the original (initial, resource) s… #19

Closed 0ric1 closed 4 years ago

0ric1 commented 4 years ago

…ize report from 3-Feb-09.

User could resize the dialog smaller as in resource defined and that causes some static text to be clipped or dissapear.

ppescher commented 4 years ago

Is this bug also present in the other resizable window classes?

0ric1 commented 4 years ago

I didn't test the other classes - we don't use these - so I don't know.

irwir commented 4 years ago

This change may cause issues with already existing codes. MakeResizable may adjusts dialog size in lpCreateStruct, and that should be taken into account when creating dialog and repositionning controls inside it.

ppescher commented 4 years ago

Looking at the changes in the proposed commit, I recall of similar issue with the order MakeResizable() is called. I think the min track size should be calculated after the window border has been changed, so I was inclined to accept this PR. But I was wondering if other classes had this same issue, because it looked like a dejavue (it might have been some comment in the CodeProject forums).

irwir commented 4 years ago

I think the min track size should be calculated after the window border has been changed, so I was inclined to accept this PR.

Currently, the client area is requested, and modified value may include frame witdth. What size it was supposed to track in the library - client area or the whole window?

For better understanding of the issue it might be useful to see the code that causes incorrect behaviour. @Blonder, would it be possible to modify one of the library's examples and show the bug?

Additional considerations. AdjustWindowRectEx

  1. Note that you cannot specify the WS_OVERLAPPED style.
  2. This API is not DPI aware And one more link:
  3. https://stackoverflow.com/questions/27928254/adjustwindowrectex-and-getwindowrect-give-wrong-size-with-ws-overlapped
0ric1 commented 4 years ago

https://www.codeproject.com/Articles/1175/ResizableLib?msg=2909968#xx2909968xx

irwir commented 4 years ago

@Blonder 2 pixel difference does not match with a thick frame width. Is there anything to fix, because a few things have changed since 2009?

Unpacking the current version 1.5.2 and building ResizableDialog demo with VS 2019 gives the following results: The first and the third dialogs cannot be resized below their initial sizes respectively. The second dialog could be resized to "caption only", but this is intentional (ResetMinTrackSize).

Would you please provide an example of the issue?

0ric1 commented 4 years ago

I have no XP any more so I cannot reproduce this, on my Windows 10 1903 with 1.5.2 it does not occure any more, but it may occure in some scenarios, if the style is modified in CResizableLayout::MakeResizable it could be that the size of the dialog changes and then the call to the SetMinTrackSize before MakeResizable is wrong because it set the wrong size.

irwir commented 4 years ago

I have no XP any more

XP in a virtual machine might be used for testing. No bugs - no fixes required.

if the style is modified CResizableLayout::MakeResizable it could be that the size of the dialog changes and then the call to the SetMinTrackSize before MakeResizable is wrong because it set the wrong size.

In that case, tracked size should be explicitly set once more. Also this should not be forgotten.

0ric1 commented 4 years ago

In that case, tracked size should be explicitly set once more.

Why should the tracked size be set twice before and after? The final size is only known after MakeResizable is called so setting it before makes no sense.

ppescher commented 4 years ago

I agree with @Blonder here: since MakeResizable can change the non-client rect, SetMinTrackSize should be called afterwards. The lib does that when the windows is first created to set a default behavior, but you can always re-define the min tracking size later.

irwir commented 4 years ago

The PR looks good to me.