Opened 7 years ago

Last modified 4 years ago

#8670 assigned enhancement

Improved zoom button behavior

Reported by: mmadia Owned by: nobody
Priority: normal Milestone: R1
Component: Applications/StyledEdit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking: #8665
Has a Patch: yes Platform: All

Description

resize a page horizontally until a horizontal scrollbar disappears, making a window as narrow as possible (with some sanity checks, of course). Vertical resizing could be to full height.

Attachments (1)

improved_zoom.patch (2.0 KB) - added by ejno1 6 years ago.
Improved zoom behavior (modified)

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by mmadia

Blocked By: 8665 removed
Blocking: 8665 added

comment:2 Changed 6 years ago by siarzhuk

Owner: changed from korli to siarzhuk
Status: newassigned

comment:3 Changed 6 years ago by ejno1

Has a Patch: set

comment:4 Changed 6 years ago by axeld

Thanks for your patch! It goes into the right direction, however, it does not deal gracefully with lines longer than the screen resolution; the StyledEdit window should never be larger than the screen.

Furthermore, I'm not sure if it really makes sense to disable word wrap, and then do the resize -- there are usually complete paragraphs in such documents. In word wrap mode, I think it makes the most sense to keep the current window width, and only resize the height to make it fit the contents. Only in non-word wrap mode, the current method should be applied for the width, but additionally, the height should be changed such that the windows is as large as needed to show all contents.

comment:5 in reply to:  4 Changed 6 years ago by ejno1

Hi axeld, thanks for the feedback.

The window shouldn't become larger than the screen because I clamp it here: min_c(max_c(width, 256.f), fullWidth)

I agree about the word wrap; I didn't think of that. I'll try to implement it when I have access to my other computer in a week or so. I did not resize vertically because of the suggestion in the ticket, but I'll add that as well.

Replying to axeld:

Thanks for your patch! It goes into the right direction, however, it does not deal gracefully with lines longer than the screen resolution; the StyledEdit window should never be larger than the screen.

Furthermore, I'm not sure if it really makes sense to disable word wrap, and then do the resize -- there are usually complete paragraphs in such documents. In word wrap mode, I think it makes the most sense to keep the current window width, and only resize the height to make it fit the contents. Only in non-word wrap mode, the current method should be applied for the width, but additionally, the height should be changed such that the windows is as large as needed to show all contents.

Changed 6 years ago by ejno1

Attachment: improved_zoom.patch added

Improved zoom behavior (modified)

comment:6 Changed 5 years ago by siarzhuk

Hi, ejno1! Thank you for your contribution!

I have tried your patch and found that window, that was zoomed cannot be un-zoomed by another click on the same tab icon. You can compare the zoom-unzoom behavior of the patched StyledEdit with the Terminal for example to observe the problem. This functionality is implemented in BWindow::Zoom() and uses the width/height of the BWindow::fPreviousFrame rect to determine if the window should be zoomed or unzoomed. In my opinion it is a good idea to utilize this base functionality and try to adjust the maxWidth and maxHeight by SetZoomLimits(..) call instead of direct overriding Zoom(...) virtual. Could you , please, try to improve your patch in this direction?

PS: BTW, is "ejno1 <none>" the real user info you want to see in Haiku commit log for your commits? I suspect "<none>" e-mail will be catched by git hooks as invalid and prevent us to push it. Could you, please provide a bit more info if possible? Thank you!

comment:7 Changed 4 years ago by siarzhuk

Owner: changed from siarzhuk to nobody

Those were taken some years ago as potential GCI tasks. Unfortunately no place is available for them in my schedule at this days.

Note: See TracTickets for help on using tickets.