Opened 10 years ago

Closed 7 years ago

#4146 closed bug (fixed)

Magnify doesn't resize nicely

Reported by: atmartens Owned by: axeld
Priority: normal Milestone: R1
Component: Applications/Magnify Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

If you make the magnify program small enough, the text runs too far outside the window, and the option button covers some of the text.

Attachments (3)

magnify-resize.png (209.8 KB) - added by atmartens 10 years ago.
The magnify program doesn't resize as it should
magnify.diff (2.2 KB) - added by negusnyul 9 years ago.
0001-Fixes-ticket-4146.patch (5.5 KB) - added by negusnyul 7 years ago.

Download all attachments as: .zip

Change History (15)

Changed 10 years ago by atmartens

Attachment: magnify-resize.png added

The magnify program doesn't resize as it should

comment:1 Changed 10 years ago by anevilyak

Component: - GeneralApplications/Magnify

comment:2 Changed 9 years ago by negusnyul

I attached a patch that automatically hides/shows the information text if needed.

Other solution: place B_AUTO_UPDATE_SIZE_LIMITS in the window constructor

however, thats not the best solution because the user cannot resize the window smaller than the default size - this is why I don't use it in the patch

Changed 9 years ago by negusnyul

Attachment: magnify.diff added

comment:3 Changed 9 years ago by negusnyul

Has a Patch: set

comment:4 Changed 8 years ago by axeld

Sorry for the very late reply; your patch has two issues that would need to be solved (I even tried, but the Magnify code is quite scary):

  • When closing the window while the info is not visible due to the size, it will lose the setting.
  • You make members public to get the preferred size of the widget. While that kind of fits to the coding style within Magnify, it would be much better to actually implement GetPreferredSize() for the info view instead, and use that (including the height of the window) to judge whether or not the info view should be visible.

comment:5 Changed 8 years ago by axeld

Has a Patch: unset

comment:6 Changed 8 years ago by negusnyul

Thanks for the reply, I will solve the mentioned issues eventually and create a new patch when I'll have the time.

comment:7 Changed 8 years ago by axeld

Great, thanks! I didn't think you could be motivated to look into this again after that many month, it's appreciated, though!

Changed 7 years ago by negusnyul

comment:8 Changed 7 years ago by negusnyul

Has a Patch: set

comment:9 Changed 7 years ago by negusnyul

It has been a long time but here is the new patch :)

(Created with git format-patch)

comment:10 in reply to:  9 Changed 7 years ago by mmadia

Replying to negusnyul:

It has been a long time but here is the new patch :)

(Created with git format-patch)

Ping, for a review. comment:4 mentions the concerns with the first patch.

comment:11 Changed 7 years ago by korli

Coding style issues:

  • isInfoTextVisible() method name should be capitalized
  • there is a space between the if and the parenthesis.
  • braces for a single line if or else block should be dropped (but not in UpdateInfoBarOnResize()).
  • the operator || shouldn't be last on a line.
  • TInfoView::GetPreferredSize() declaration variable names don't match the implementation ones.

comment:12 Changed 7 years ago by mmadia

Resolution: fixed
Status: newclosed

Thanks for the patch negusnyul! Applied in hrev45301, along with korli's code review.

Note: See TracTickets for help on using tickets.