Closed
Bug 984140
Opened 10 years ago
Closed 9 years ago
UITour: Info panels with no image are missing a left margin
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
People
(Reporter: MattN, Assigned: Dolske)
References
Details
(Whiteboard: [Australis:P4])
Attachments
(3 files, 1 obsolete file)
112.86 KB,
image/png
|
Details | |
492.16 KB,
image/png
|
Details | |
1.50 KB,
patch
|
Dolske
:
approval-mozilla-aurora+
Dolske
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Since the spec only had panels with icons and buttons, I forgot to consider when those aren't provided. I've pushed the UITour catalogue to my gh-pages branch so it's easier to manually test other combinations in the future: http://mnoorenberghe.github.io/mozilla-uitour/demos/catalogue.html Luckily I don't know of any plans to use such an info panel for 29 but we should probably fix this just in case.
Reporter | ||
Comment 1•10 years ago
|
||
I'm not working on this at the moment but I think it would be valuable to fix for future tour info panels.
Assignee: MattN+bmo → nobody
Status: ASSIGNED → NEW
Flags: firefox-backlog+
Comment 2•9 years ago
|
||
Hi Matt, just a friendly +1 to get this fixed at some point in the future. For the last iterations of various tours the copy/design folks expressed the wish to use info panels without an image, but we had to find something suitable to workaround this layout bug.
Reporter | ||
Updated•9 years ago
|
Points: --- → 2
Comment 3•9 years ago
|
||
This bug will affect our design/copy for the Hello UITour. I'm not adding as a blocker yet, but I would like to know if this is something we can land in the next week. Thanks
Flags: needinfo?(mreavy)
Flags: needinfo?(dolske)
Reporter | ||
Comment 4•9 years ago
|
||
Removing needinfo from mreavy since this is a non-Hello change. I made it block the meta bug since it's wanted although not a hard blocker it seems.
Blocks: fx-UITour-Hello
Flags: needinfo?(mreavy)
Comment 5•9 years ago
|
||
(In reply to Cory Price [:ckprice] from comment #3) > This bug will affect our design/copy for the Hello UITour. This isn't a blocker and comment 2 suggests there's already a workaround in place. "Nice to have" but we're not going to worry about it if it introduces complication.
No longer blocks: fx-UITour-Hello
Flags: needinfo?(dolske)
Assignee | ||
Comment 6•9 years ago
|
||
Trivial fix. The negative margin on the icon container was shifting things over, so just hide the container instead of the icon itself.
Assignee: nobody → dolske
Attachment #8537003 -
Flags: review?(bmcbride)
Assignee | ||
Comment 7•9 years ago
|
||
Reporter | ||
Comment 8•9 years ago
|
||
Comment on attachment 8537003 [details] [diff] [review] Patch v.1 Review of attachment 8537003 [details] [diff] [review]: ----------------------------------------------------------------- rs=me with the below fixed (assuming it works with and without the icon specified) ::: browser/modules/UITour.jsm @@ +1168,5 @@ > > tooltipTitle.textContent = aTitle || ""; > tooltipDesc.textContent = aDescription || ""; > tooltipIcon.src = aIconURL || ""; > + tooltipContainer.hidden = !aIconURL; I'm confused. Did you forget to qrefresh? You defined `tooltipIconContainer` but are referencing `tooltipContainer`?
Attachment #8537003 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 9•9 years ago
|
||
Yeah, dunno what happened there. Bad cut'n'paste when updating my tree I guess. https://hg.mozilla.org/integration/fx-team/rev/889224b8a0fd [Triage Comment] Approving for aurora+beta since this is needed for the Hello tour in fx35 and is trivial and low-risk.
Attachment #8537003 -
Attachment is obsolete: true
Attachment #8537573 -
Flags: approval-mozilla-beta+
Attachment #8537573 -
Flags: approval-mozilla-aurora+
Comment 10•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a51fc9b5868 https://hg.mozilla.org/releases/mozilla-beta/rev/6c84b644fb37
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/889224b8a0fd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•9 years ago
|
Iteration: --- → 37.2
Flags: qe-verify?
Reporter | ||
Updated•9 years ago
|
Flags: qe-verify? → qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•