Closed
Bug 983653
Opened 11 years ago
Closed 11 years ago
UITour: Make the highlight effect a circle on the bookmarks combo button
Categories
(Firefox :: General, defect)
Tracking
()
VERIFIED
FIXED
Firefox 30
People
(Reporter: bwinton, Assigned: bwinton)
References
Details
(Whiteboard: [Australis:P4+])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bwinton
:
review+
bwinton
:
ui-review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The previous patch was meant to make the bookmarks combo button have a circular highlight (and I _swear_ it did when I tested it), but apparently on nightly it doesn't anymore, so this patch is here to give me a chance to fix that.
Assignee | ||
Comment 1•11 years ago
|
||
The bookmarks button now has a ratio of 2.6363636363636362 (instead of the 2.01) it had when I wrote the patch), so I think "3" will give us enough room to handle any future padding changes.
Assignee: nobody → bwinton
Attachment #8391202 -
Flags: ui-review?(shorlander)
Attachment #8391202 -
Flags: review?(MattN+bmo)
Comment 2•11 years ago
|
||
Comment on attachment 8391202 [details] [diff] [review]
bug983653.patch
Review of attachment 8391202 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me!
Attachment #8391202 -
Flags: ui-review?(shorlander) → ui-review+
Comment 3•11 years ago
|
||
Comment on attachment 8391202 [details] [diff] [review]
bug983653.patch
Review of attachment 8391202 [details] [diff] [review]:
-----------------------------------------------------------------
Holly is also fine with trying this.
::: browser/modules/UITour.jsm
@@ +810,5 @@
> let highlightWidth = targetRect.width;
> let minDimension = Math.min(highlightHeight, highlightWidth);
> let maxDimension = Math.max(highlightHeight, highlightWidth);
>
> // If the dimensions are within 110% of each other (to include the bookmarks button),
You forgot to update the comment again :)
Attachment #8391202 -
Flags: review?(MattN+bmo) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Fixed the comment. :)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): (I can't find it, but something changed the padding or margin on the bookmarks button.)
User impact if declined: Less noticeable bookmarks highlight in the UI tour.
Testing completed (on m-c, etc.): Manual. Also the video in bug 983737.
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None.
Attachment #8391202 -
Attachment is obsolete: true
Attachment #8391334 -
Flags: ui-review+
Attachment #8391334 -
Flags: review+
Attachment #8391334 -
Flags: approval-mozilla-aurora?
Comment 5•11 years ago
|
||
I will approve it once it is in m-c.
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 6•11 years ago
|
||
Status: NEW → ASSIGNED
status-firefox29:
--- → affected
status-firefox30:
--- → affected
Keywords: checkin-needed
Summary: UI Tour: Fix shape of UI highlight effect on Bookmarks combo button. → UITour: Make the highlight effect a circle on the bookmarks combo button
Whiteboard: [Australis:P4+] → [Australis:P4+][fixed-in-fx-team]
Comment 7•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4+][fixed-in-fx-team] → [Australis:P4+]
Target Milestone: --- → Firefox 30
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #8391334 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 8•11 years ago
|
||
Updated•11 years ago
|
Comment 9•11 years ago
|
||
Verified as fixed using the following environment:
Firefox Aurora 29.0a2
Build Id:20140317004002
OS: Win 8.1 x 32, Mac Os x 10.8.5, Ubuntu 13.04 x64
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•