Opened 8 years ago

Closed 4 years ago

Last modified 4 years ago

#7394 closed enhancement (fixed)

Display zoom factor

Reported by: humdinger Owned by: waddlesplash
Priority: normal Milestone: Unscheduled
Component: Applications/ShowImage Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

This is hrev41068.

The status bar could have a zoom factor display. I think it was even mentioned last BeGeistert. Even better: make it a pop-up menu that displays the current zoom factor and offers other factors for zooming.

BTW, what's with the Tracker-like click behaviour of the status bar? Anyone using it?
It could be rewired to show extended file info instead...

Attachments (8)

Before.png (156.5 KB ) - added by Janus 4 years ago.
After.png (132.8 KB ) - added by Janus 4 years ago.
Without icons.png (133.5 KB ) - added by Janus 4 years ago.
StyleEdit Separetor.png (126.7 KB ) - added by Janus 4 years ago.
Comma.png (105.9 KB ) - added by Janus 4 years ago.
0001-Fix-7394.-Add-Zoom-level-to-StatusBar.-Improve-graph.patch (12.3 KB ) - added by Janus 4 years ago.
Patch Result.png (94.5 KB ) - added by Janus 4 years ago.
0001-Fix-7394.-Add-Zoom-level-to-StatusBar.-Improve-graph.2.patch (12.7 KB ) - added by Janus 4 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 by axeld, 8 years ago

FWIW I like the Tracker navigation menu (likewise in Pe), and I find myself using it quite often.

comment:2 by humdinger, 8 years ago

I didn't even know about that (in Pe neither!)... Probably, because usually I have the Tracker window open from where I launched the image in ShowImage and go back to that for navigation. But I guess it may be convenient...
So, still... a zoom display/setting down there would be nice.

comment:3 by leavengood, 8 years ago

I have made a simple change locally which adds some text in the status bar for the current zoom percentage, which is a nice addition.

As for your full suggestion: rather than a drop down in the status bar, it seems like a text-box in the tool bar would work nicely as well. Or a text-box with a drop down menu next to it with some even-number zoom factors (25%, 50%, 75%, 125%, ..., 400%.) Since the zoom buttons are there already in the toolbar it seems like a good place.

comment:4 by humdinger, 8 years ago

One thing speaking against the zoom level display/setting in the toolbar is that it disrupts the visuals a bit, because it's the only non-icon button. Also, I think changing to a specific zoom level isn't needed very often. Most of the time you just resize the image window until it's convenient and at max. use the zoom in/out button to reach the next even-numbered zoom level.
So, putting the zoom level as info to the rest of the data in the status bar seems appropriate. Changing levels via pop-up menu would be just a nice gimmick.
What I wouldn't like at all is a white free text field to enter a specific zoom level. Besides being used not often (IMO), I don't think it'll look right. If deemed absolutely necessary, add a "Other..." to the predefined levels and open a small window to enter a number. Also, don't use too many predefines.

comment:5 by halilpk, 8 years ago

(GCI-2011 Participant)

Haiku revision: hrev42211 still a bug. There is no zoom factor on the ShowImage window or menu . System: Haiku hrev1-alpha3 on VMware workstation 8 on windows 7 32 bit

by Janus, 4 years ago

Attachment: Before.png added

by Janus, 4 years ago

Attachment: After.png added

comment:6 by Janus, 4 years ago

Thoughts?

comment:7 by humdinger, 4 years ago

Looks good to me! Though I don't know if the icons are really necessary. "1982x882" is obviously the size and the "%" is a clear indication of zoom level.

by Janus, 4 years ago

Attachment: Without icons.png added

comment:8 by humdinger, 4 years ago

I like it, though some sort of delimiter would be nice. Either commas, or maybe even better: a vertical separator line, so it looks like the "Login" menu at the far right in HaikuDepot's menubar, if that's possible.

by Janus, 4 years ago

Attachment: StyleEdit Separetor.png added

by Janus, 4 years ago

Attachment: Comma.png added

comment:9 by Janus, 4 years ago

StyledEdit Separator and Comma separator mockup.

comment:10 by humdinger, 4 years ago

+1 for the separator version. nice mockup, where's the patch? :D

comment:11 by Janus, 4 years ago

ASAP!

comment:12 by Janus, 4 years ago

Has a Patch: set

comment:13 by Janus, 4 years ago

The status bar is based on StyledEdit.

by Janus, 4 years ago

Attachment: Patch Result.png added

comment:14 by humdinger, 4 years ago

Wow, fast delivery. :)
To my amateurish eye it looks good. Hope an actual dev can look it over and apply it soon. Thanks!

comment:15 by waddlesplash, 4 years ago

Milestone: R1Unscheduled
Owner: changed from leavengood to waddlesplash
Status: newassigned

Looks like there's a lot of unnecessary logic that's added in the patch due to ShowImage not using layouts... I'll poke at this once I fix the layout regressions in Tracker.

comment:16 by Janus, 4 years ago

The code is the same of StyledEdit, after the patch the code is simpler than the actual.

comment:17 by Janus, 4 years ago

I hope someday someone applies the patch of #7590 1 year 1 line of code...

comment:18 by Janus, 4 years ago

I think it is disrespectful for the person that post a fix to see that the code (code already present in the repository) is not good enough. What was the alternative, convert showimage to layouts a megapatch? If 1 line of code took a year, a big patch? I think the question should be if the patch improve the software. Which are the benefits for the end user to convert showimage to layouts? etc.

This is my honest opinion.

comment:19 by waddlesplash, 4 years ago

What was the alternative, convert showimage to layouts a megapatch? If 1 line of code took a year, a big patch?

No, I thought I'd do the work to convert ShowImage to layouts. Adding a lot of complexity is not something I'm confident in applying. If someone else approves the patch, fine with me, but I'm not confident enough to do so.

comment:20 by axeld, 4 years ago

Hey Janus, I think your patch looks good. I have just two suggestions:

  1. Instead of the relatively strange way to set the zoom text, why not use BString::SetToFormat()?
  2. Also, instead of duplicating the code to do so, please use a private method that updates it instead.

Also, I spotted a minor style violation in that line: there is no space after the cast.

It would be nice if you could give me a short notice if you are willing to do these changes, otherwise I would apply your patch as is, and do them myself.

comment:21 by Janus, 4 years ago

For symmetry there is a private method for each field. Only for the zoom was very ugly. I didn't knew the existence of BString::SetToFormat :-(

comment:22 by waddlesplash, 4 years ago

Resolution: fixed
Status: assignedclosed

Janus fixed this in hrev49007 but he forgot to close the ticket.

comment:23 by Janus, 4 years ago

I committed the patch 2 minutes ago... You are too fast!

Note: See TracTickets for help on using tickets.