Closed
Bug 1100565
Opened 10 years ago
Closed 9 years ago
Margins and paddings are inconsistent/broken since bug 1074672
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35 fixed, firefox36 fixed)
backlog | Fx35+ |
People
(Reporter: jaws, Assigned: jaws)
References
Details
(Keywords: regression, Whiteboard: [UX bug])
Attachments
(4 files, 6 obsolete files)
10.05 KB,
image/png
|
Details | |
16.90 KB,
image/png
|
Details | |
2.92 KB,
patch
|
mikedeboer
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
6.50 KB,
patch
|
mikedeboer
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
See attached screenshot. The "No current conversations" overlaps the "Beta" tag, and the terms of service are touching the borders of the panel. Further, the "No current conversations" is indented .5rem, but the "Guest" string is indented 14px. Same for "Available" and the available-dropdown.
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
The rooms list is also inconsistent.
Comment 2•10 years ago
|
||
jared going to look at this one - pretty ugly.
backlog: --- → Fx35+
Priority: -- → P1
Whiteboard: [UX bug]
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 36.3
Points: --- → 8
Flags: qe-verify-
Flags: firefox-backlog+
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8526229 -
Flags: review?(mdeboer)
Assignee | ||
Comment 4•10 years ago
|
||
Fixed a margin under the Start a Conversation button.
Attachment #8526229 -
Attachment is obsolete: true
Attachment #8526229 -
Flags: review?(mdeboer)
Attachment #8526256 -
Flags: review?(mdeboer)
Assignee | ||
Comment 5•10 years ago
|
||
Sorry, a line crept in from another patch.
Attachment #8526256 -
Attachment is obsolete: true
Attachment #8526256 -
Flags: review?(mdeboer)
Attachment #8526260 -
Flags: review?(mdeboer)
Comment 6•10 years ago
|
||
Comment on attachment 8526260 [details] [diff] [review] Patch v1.1.1 Review of attachment 8526260 [details] [diff] [review]: ----------------------------------------------------------------- Pretty big, risky patch, but overall this looks like the right direction. You have to iterate it once more, though, to make sure that the footer stays where it should be in the rooms view (see attached screenshot)
Attachment #8526260 -
Flags: review?(mdeboer) → review-
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
I added back the min-height.
Attachment #8526260 -
Attachment is obsolete: true
Attachment #8526789 -
Attachment is obsolete: true
Attachment #8526930 -
Flags: review?(mdeboer)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8526930 -
Attachment is obsolete: true
Attachment #8526930 -
Flags: review?(mdeboer)
Attachment #8526983 -
Flags: review?(mdeboer)
Assignee | ||
Comment 10•10 years ago
|
||
From your request in bug 1101754, what do you think about this patch? Here is a screenshot (partner logo covered), http://screencast.com/t/CeJ1jXK4v This doesn't vertically center the contents, but moves the ToS section to be aligned to the bottom of the panel. I couldn't find a way to get these contents vertically centered, but this looks okay to me.
Attachment #8527009 -
Flags: review?(mdeboer)
Comment 11•9 years ago
|
||
Comment on attachment 8526983 [details] [diff] [review] Patch v1.2 (rebased) Review of attachment 8526983 [details] [diff] [review]: ----------------------------------------------------------------- One issue left: when you hover a contact, the call button does not cover the 'Google' icon, which is visible for imported contacts. ::: browser/components/loop/content/shared/css/panel.css @@ +233,5 @@ > } > > .room-list > .room-entry > p { > margin: 0; > + padding: .2em 0; Shouldn't this be `rem` too?
Attachment #8526983 -
Flags: review?(mdeboer) → review-
Comment 12•9 years ago
|
||
Comment on attachment 8527009 [details] [diff] [review] Patch part 2 (fix vertical alignment for Getting Started + ToS view) Review of attachment 8527009 [details] [diff] [review]: ----------------------------------------------------------------- LGTM :)
Attachment #8527009 -
Flags: review?(mdeboer) → review+
Assignee | ||
Comment 13•9 years ago
|
||
With the Google icon fixed so it hides behind the action button when the contact is hovered, and the margins switched from em to rem on the .room-list > .room-entry > p.
Attachment #8526983 -
Attachment is obsolete: true
Attachment #8527951 -
Flags: review?(mdeboer)
Updated•9 years ago
|
Iteration: 36.3 → 37.1
Comment 14•9 years ago
|
||
Comment on attachment 8527951 [details] [diff] [review] Patch v1.2.1 Review of attachment 8527951 [details] [diff] [review]: ----------------------------------------------------------------- Ship it!
Attachment #8527951 -
Flags: review?(mdeboer) → review+
Assignee | ||
Comment 15•9 years ago
|
||
Folded both patches together, https://hg.mozilla.org/integration/fx-team/rev/2e46648a150c
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2e46648a150c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 17•9 years ago
|
||
Comment on attachment 8527951 [details] [diff] [review] Patch v1.2.1 Approval Request Comment [Feature/regressing bug #]: bug 1074672 [User impact if declined]: Visual bugs (overlaps, etc) - see images [Describe test coverage new/current, TBPL]: manual testing; easily visible [Risks and why]: Low risk - purely visual CSS changes and adding an id to a div [String/UUID change made/needed]: none
Attachment #8527951 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8527009 -
Flags: approval-mozilla-aurora?
Comment 18•9 years ago
|
||
Is it possible to have automated tests (maybe a reftest) to cover this, since it is a regression?
Assignee | ||
Comment 19•9 years ago
|
||
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #18) > Is it possible to have automated tests (maybe a reftest) to cover this, > since it is a regression? Not really. We have dealt with similar issues in other places of the Firefox UI, and due to platform and environment differences we're unable to do things like reftests for chrome UI reliably.
Comment 20•9 years ago
|
||
Comment on attachment 8527009 [details] [diff] [review] Patch part 2 (fix vertical alignment for Getting Started + ToS view) Approval Request Comment Move request to beta
Attachment #8527009 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Updated•9 years ago
|
Attachment #8527951 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Updated•9 years ago
|
Attachment #8527009 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•9 years ago
|
Attachment #8527951 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•9 years ago
|
Updated•9 years ago
|
Iteration: 37.1 → 36.3
Updated•9 years ago
|
Iteration: 36.3 → 37.1
You need to log in
before you can comment on or make changes to this bug.
Description
•