Closed
Bug 1106010
Opened 9 years ago
Closed 9 years ago
"Beta" tag overlaps the Loop panel heading
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35+ verified, firefox36+ verified, firefox37 verified)
backlog | Fx35+ |
People
(Reporter: Gijs, Assigned: mikedeboer)
References
Details
(Whiteboard: [rooms])
Attachments
(3 files)
106.05 KB,
image/png
|
Details | |
8.27 KB,
patch
|
NiKo
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
99.49 KB,
image/jpeg
|
Details |
The "beta" banner overlaps the text ("no current notifications"), and there is random whitespace at the bottom. This is on current nightly (Nov. 27).
Comment hidden (off-topic) |
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mdeboer
Severity: normal → blocker
Status: NEW → ASSIGNED
Iteration: --- → 37.1
Points: --- → 2
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Whiteboard: [rooms]
Assignee | ||
Updated•9 years ago
|
Summary: Hello panel looks broken on OS X → Hello panel looks broken with an empty rooms list when connected as Guest
Reporter | ||
Comment 2•9 years ago
|
||
fwiw, re the summary change, it looks just as broken before clicking the broken 'get started' link...
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Updated•9 years ago
|
Flags: needinfo?(mreavy)
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Comment hidden (obsolete) |
Reporter | ||
Updated•9 years ago
|
Status: REOPENED → ASSIGNED
Comment 10•9 years ago
|
||
[Tracking Requested - why for this release]: We need this fix for "Rooms", which is already enabled in Fx35.
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Updated•9 years ago
|
backlog: --- → Fx35+
Comment 11•9 years ago
|
||
Hi Mike, Is this a bug you're able to do this week so we can uplift into Beta Monday for testing?
Flags: needinfo?(mdeboer)
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 9 years ago
Flags: needinfo?(mdeboer)
Resolution: --- → DUPLICATE
Comment 13•9 years ago
|
||
Matt -- Given the original report, why is this a dup of bug 1089191? Original report: The "beta" banner overlaps the text ("no current notifications"), and there is random whitespace at the bottom. This is on current nightly (Nov. 27).
Flags: needinfo?(MattN+bmo)
Comment 14•9 years ago
|
||
Gijs told me about this bug in-person and mentioned "random whitespace at the bottom" which is what 1089191 is about. This bug ended up talking about three different issues. I'll re-open for the Beta tag overlap.
Blocks: 1076709
Status: RESOLVED → REOPENED
Flags: needinfo?(MattN+bmo) → needinfo?(mdeboer)
Resolution: DUPLICATE → ---
Summary: Hello panel looks broken with an empty rooms list when connected as Guest → "Beta" tag overlaps the Loop panel heading
Updated•9 years ago
|
Flags: qe-verify?
Flags: firefox-backlog+
Comment 15•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #3) > The link I get is: > https://www.mozilla.org/en-US/firefox/36.0a1/hello/start?utm_source=firefox-browser&utm_medium=firefox-browser&utm_campaign=getting-started > > which is a 404. The "Get Started" redirect is now live.
Comment 16•9 years ago
|
||
Are we still keeping the Beta tag for Fx36? When can we drop the Beta tag?
Comment 17•9 years ago
|
||
I feel we should keep the Beta tag until we're confident that the product is solid and polished. I don't think we can tie that to a release but to success criteria for the product. I want to talk more this week about that.
Updated•9 years ago
|
Status: REOPENED → ASSIGNED
Updated•9 years ago
|
Iteration: 37.1 → 37.2
Assignee | ||
Comment 18•9 years ago
|
||
Flags: needinfo?(mdeboer)
Attachment #8533709 -
Flags: review?(nperriault)
Assignee | ||
Updated•9 years ago
|
Flags: qe-verify? → qe-verify+
Comment on attachment 8533709 [details] [diff] [review] Patch v1: make sure enough UI elements are always visible when not signed in to FxA Review of attachment 8533709 [details] [diff] [review]: ----------------------------------------------------------------- Looks better! Though my journey through this part of the Panel code made me cringe a little :) (I think [tech-debt] bugs for proper refactoring are already filed anyway) ::: browser/components/loop/content/js/panel.jsx @@ +928,5 @@ > } > > + // Determine which buttons to NOT show. > + var hideButtons = []; > + if (!this.state.userProfile && !this.props.showTabButtons) Side note: I was intrigued and checked what this `showTabButtons` prop was used for; it seems to be only used in panel_test.js to "force rendering" the buttons in this context… Boy, this part needs refactor. At least we should comment about it in the appropriate propTypes section. @@ +929,5 @@ > > + // Determine which buttons to NOT show. > + var hideButtons = []; > + if (!this.state.userProfile && !this.props.showTabButtons) > + hideButtons.push("contacts"); Nit: I think we've been avoiding omitting curly brackets for single line statements so far. ::: browser/components/loop/content/shared/css/panel.css @@ +192,5 @@ > } > > .room-list { > max-height: 335px; /* XXX better computation needed */ > + min-height: 80px; /* Show at least the space for one element */ It's matter of taste really, but my personal tests indicated 7px as a better value — see upcoming screenshot.
Attachment #8533709 -
Flags: review?(nperriault) → review+
Assignee | ||
Comment 21•9 years ago
|
||
(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #20) > Created attachment 8533987 [details] > Suggested css modifications OK, you win :) Thanks for the review!
Assignee | ||
Comment 22•9 years ago
|
||
Pushed to fx-team as https://hg.mozilla.org/integration/fx-team/rev/9386c1c866cf I will file a follow-up bug to fix the SocialAPI panel frame sizing code to allow for a different min-height/ min-width.
Comment 23•9 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #22) > I will file a follow-up bug to fix the SocialAPI panel frame sizing code to > allow for a different min-height/ min-width. Bug 1089191 could handle this
Comment 24•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9386c1c866cf
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 25•9 years ago
|
||
The blue icon looks to me a little odd in the middle of the panel http://i.imgur.com/P23gCMz.png Also, any idea why the conversation list got unsorted ?
(In reply to Paul Silaghi, QA [:pauly] from comment #25) > The blue icon looks to me a little odd in the middle of the panel > http://i.imgur.com/P23gCMz.png Well, we had no UX spec for this situation and we're just engineers :p NI :sevaan here. > Also, any idea why the conversation list got unsorted ? This is by design, they're sorted by latest activity ctime desc.
Flags: needinfo?(sfranks)
Comment 27•9 years ago
|
||
Anyway, marking verified since the initial problem is fixed. 37.0a1 (2014-12-11) Win 7
Status: RESOLVED → VERIFIED
status-firefox37:
--- → verified
Comment 28•9 years ago
|
||
(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #26) > (In reply to Paul Silaghi, QA [:pauly] from comment #25) > > The blue icon looks to me a little odd in the middle of the panel > > http://i.imgur.com/P23gCMz.png > > Well, we had no UX spec for this situation and we're just engineers :p NI > :sevaan here. I would rather that row just not be displayed unless the user is logged in.
Flags: needinfo?(sfranks)
Comment 29•9 years ago
|
||
Comment on attachment 8533709 [details] [diff] [review] Patch v1: make sure enough UI elements are always visible when not signed in to FxA Approval Request Comment [Feature/regressing bug #]: beta tag [User impact if declined]: Beta tag can partly cover UI elements when a Guest (which is the default) [Describe test coverage new/current, TBPL]: on m-c for several days; manually checked. [Risks and why]: low risk - adjusts positioning and tweaks logic for button hiding. [String/UUID change made/needed]: none
Attachment #8533709 -
Flags: approval-mozilla-beta?
Attachment #8533709 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox35:
--- → affected
status-firefox36:
--- → affected
Updated•9 years ago
|
Attachment #8533709 -
Flags: approval-mozilla-beta?
Attachment #8533709 -
Flags: approval-mozilla-beta+
Attachment #8533709 -
Flags: approval-mozilla-aurora?
Attachment #8533709 -
Flags: approval-mozilla-aurora+
Comment 32•9 years ago
|
||
Verified fixed 36.0a2 (2014-12-17), 35b4 Win 7
You need to log in
before you can comment on or make changes to this bug.
Description
•