#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: | ||
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)
Change History (31)
comment:1 by , 14 years ago
comment:2 by , 14 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 , 13 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 , 13 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 , 13 years ago
by , 10 years ago
Attachment: | Before.png added |
---|
by , 10 years ago
comment:7 by , 10 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 , 10 years ago
Attachment: | Without icons.png added |
---|
comment:8 by , 10 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 , 10 years ago
Attachment: | StyleEdit Separetor.png added |
---|
by , 10 years ago
by , 10 years ago
Attachment: | 0001-Fix-7394.-Add-Zoom-level-to-StatusBar.-Improve-graph.patch added |
---|
comment:12 by , 10 years ago
patch: | 0 → 1 |
---|
by , 10 years ago
Attachment: | Patch Result.png added |
---|
comment:14 by , 10 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 , 10 years ago
Milestone: | R1 → Unscheduled |
---|---|
Owner: | changed from | to
Status: | new → assigned |
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 , 10 years ago
The code is the same of StyledEdit, after the patch the code is simpler than the actual.
comment:17 by , 10 years ago
I hope someday someone applies the patch of #7590 1 year 1 line of code...
comment:18 by , 10 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 , 10 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 , 10 years ago
Hey Janus, I think your patch looks good. I have just two suggestions:
- Instead of the relatively strange way to set the zoom text, why not use BString::SetToFormat()?
- 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.
by , 10 years ago
Attachment: | 0001-Fix-7394.-Add-Zoom-level-to-StatusBar.-Improve-graph.2.patch added |
---|
comment:21 by , 10 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 , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Janus fixed this in hrev49007 but he forgot to close the ticket.
FWIW I like the Tracker navigation menu (likewise in Pe), and I find myself using it quite often.