Closed
Bug 710762
Opened 13 years ago
Closed 13 years ago
Keyboard control for zoom in tilt (page inspector 3D)
Categories
(DevTools :: General, defect)
Tracking
(firefox11+ verified)
VERIFIED
FIXED
Firefox 12
People
(Reporter: dangoor, Assigned: vporof)
Details
(Keywords: verified-beta, Whiteboard: [tilt][fixed-in-fx-team][qa!])
Attachments
(3 files, 1 obsolete file)
10.64 KB,
patch
|
Details | Diff | Splinter Review | |
12.85 KB,
patch
|
Details | Diff | Splinter Review | |
12.84 KB,
patch
|
rcampbell
:
review+
christian
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Victor tells me that there's no way right now to zoom using just the keyboard in 3D mode in the page inspector. It would be good to have one. The = and - keys seem like good options (for zoom in/out, respectively). Supporting shift+those keys would be good as well, so that the user could push + for zoom in.
Assignee | ||
Updated•13 years ago
|
Whiteboard: [tilt]
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #581938 -
Flags: review?(rcampbell)
Comment 2•13 years ago
|
||
Comment on attachment 581938 [details] [diff] [review] v1 + if (str === "+" || str === "=") { + this.arcball.mouseScroll(-ARCBALL_TRANSLATION_STEP); + } else if (str === "-" || str === "_") { + this.arcball.mouseScroll(ARCBALL_TRANSLATION_STEP); + } not sure all non-US keyboards have this arrangement. see http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#471 Should probably mimic those keys. Also, would be good to have a zoom reset key on Cmd/ctrl-0. We should probably verify this with a test as well.
Attachment #581938 -
Flags: review?(rcampbell) → review-
Assignee | ||
Comment 3•13 years ago
|
||
Don't ask me how I write these tests.
Attachment #581938 -
Attachment is obsolete: true
Attachment #582506 -
Flags: review?(rcampbell)
Assignee | ||
Comment 4•13 years ago
|
||
It would also probably best if zooming via keyboard and mouse would be unified in the arcball implementation. Added fix in here because a new bug would seem too much for this.
Attachment #582518 -
Flags: review?(rcampbell)
Comment 5•13 years ago
|
||
Comment on attachment 582518 [details] [diff] [review] v3 /** * Object retaining the current pressed key codes. @@ -1114,6 +1109,7 @@ this._lastTrans = vec3.create(); this._deltaTrans = vec3.create(); this._currentTrans = vec3.create(aInitialTrans); + this._zoomAmmount = 0; zoomAmount (typo!) - let scrollValue = this._scrollValue; + let zoomAmmount = this._zoomAmmount; let keyCode = this._keyCode; + if (keyCode[Ci.nsIDOMKeyEvent.DOM_VK_I] || + keyCode[Ci.nsIDOMKeyEvent.DOM_VK_ADD] || + keyCode[Ci.nsIDOMKeyEvent.DOM_VK_EQUALS]) { these are going to be impossible to localize. Kind of the reason I wanted to sniff the keycodes off of the document's page zoom keys. Either that or put them in a properties file. Certainly for I, O and R those keys may not make sense in other locales. +/- and = may be ok, but I've never really looked to see how they get localized elsewhere. It also looks like we could replace all the previous if(key) checks with a switch statement. + * Function handling the arcball zoom ammount. pesky typo again. So, typos fixed, not fully sure what to do about the l10n issue. Might be ok for a follow-up. Axel, do you have any recommendations for zoom key controls? Is sniffing the DOM for the current zoom keys an acceptable solution or should we bundle these into a properties file? Thanks.
Attachment #582518 -
Flags: review?(l10n)
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to Rob Campbell [:rc] (robcee) from comment #5) > > these are going to be impossible to localize. Kind of the reason I wanted to > sniff the keycodes off of the document's page zoom keys. Either that or put > them in a properties file. > > Certainly for I, O and R those keys may not make sense in other locales. +/- > and = may be ok, but I've never really looked to see how they get localized > elsewhere. > Then maybe we should localize ALL the keys? (even for up/down/left/right and rotations..) > > So, typos fixed, not fully sure what to do about the l10n issue. Might be ok > for a follow-up. > > Axel, do you have any recommendations for zoom key controls? Is sniffing the > DOM for the current zoom keys an acceptable solution or should we bundle > these into a properties file? Thanks. Afaik, sniffing would be bad because it returns a string ("+" for zoom, etc.). It's not safe to get the correct keycode from that. (For example, on my keyboard "-" is interpreted as the same keycode for DOM_VK_INSERT, and doing fromCharCode returns "M" for the keyCode I get when I press "-").
Comment 7•13 years ago
|
||
hm, yeah. OK. Maybe we can file a bug to make the other keys localizeable. Want to file a bug for that? In the meantime we can get this in for this release (with the typo fixed).
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•13 years ago
|
||
Fixed typo. Followup bug for localization would be perfect.
Attachment #582816 -
Flags: review?(rcampbell)
Comment 9•13 years ago
|
||
Comment on attachment 582816 [details] [diff] [review] v4 ok!
Attachment #582816 -
Flags: review?(rcampbell) → review+
Comment 10•13 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/de1cf77a8d6b
tracking-firefox11:
--- → ?
Whiteboard: [tilt] → [tilt][fixed-in-fx-team]
Comment 11•13 years ago
|
||
Comment on attachment 582816 [details] [diff] [review] v4 This would be nice to get into aurora. It adds page zoom controls on +, -, I, O keys. It even has a test!
Attachment #582816 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 12•13 years ago
|
||
For the record, there's the followup bug 712026 for localization.
Comment 13•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/de1cf77a8d6b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Comment 14•13 years ago
|
||
Comment on attachment 582816 [details] [diff] [review] v4 [triage comment] Approved for aurora. While this probably wouldn't meet our criteria at the end of a cycle ("nice to have", feature works fine without, etc). We are early in the cycle and the risk looks low to take this for what will likely be a banner feature.
Attachment #582816 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•13 years ago
|
Attachment #582518 -
Flags: review?(l10n)
Comment 15•13 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/37d9a6bb1156
status-firefox11:
--- → fixed
Updated•13 years ago
|
Comment 16•12 years ago
|
||
Comment on attachment 582518 [details] [diff] [review] v3 clearing old requests.
Attachment #582518 -
Flags: review?(rcampbell)
Updated•12 years ago
|
Attachment #582506 -
Flags: review?(rcampbell)
Comment 17•12 years ago
|
||
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0a2) Gecko/20120206 Firefox/12.0a2 Verified the fix on the latest beta build (11.0 beta 1). "-"/"=", "I"/"O", "-"/"+" keys support zooming in and out in 3D mode in the page inspector.
Status: RESOLVED → VERIFIED
Keywords: verified-beta
Whiteboard: [tilt][fixed-in-fx-team][qa+] → [tilt][fixed-in-fx-team][qa!]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•