Closed
Bug 1162905
Opened 9 years ago
Closed 9 years ago
Implement layout changes for context view in the rooms list
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox40 verified, firefox41 verified)
People
(Reporter: mikedeboer, Assigned: mikedeboer)
References
Details
(Whiteboard: [context][ux bug])
Attachments
(4 files, 6 obsolete files)
606.02 KB,
application/pdf
|
Details | |
120.16 KB,
image/png
|
Details | |
16.07 KB,
patch
|
mikedeboer
:
review+
|
Details | Diff | Splinter Review |
14.81 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Inside the panel we allow attaching a context to a new conversation/ room. Sevaan has provided some layout updates (see attachment) to this view inside the Loop panel, which we should implement.
Flags: qe-verify+
Flags: firefox-backlog+
Assignee | ||
Comment 1•9 years ago
|
||
Updated•9 years ago
|
Rank: 9
Whiteboard: [context][ux bug]
Updated•9 years ago
|
Iteration: 40.3 - 11 May → 41.1 - May 25
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8605216 -
Flags: review?(standard8)
Comment 4•9 years ago
|
||
Comment on attachment 8605216 [details] [diff] [review] Patch v1: UX refresh for the available context area Review of attachment 8605216 [details] [diff] [review]: ----------------------------------------------------------------- I've just tried testing this on top of bug 1162903 and it seems I can't get the checkbox to toggle anymore. Nothing in the error console. ::: browser/components/loop/content/js/panel.jsx @@ -772,5 @@ > <div className={contextClasses}> > - <label className="context-enabled"> > - <input className="context-checkbox" > - type="checkbox" onChange={this.onCheckboxChange}/> > - {mozL10n.get("context_offer_label")} We should remove the string for this from the properties file. We might need a separate patch for uplift, but that should be easy.
Attachment #8605216 -
Flags: review?(standard8) → review-
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8605216 -
Attachment is obsolete: true
Flags: needinfo?(mdeboer)
Attachment #8605260 -
Flags: review?(standard8)
Assignee | ||
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
Thanks. The text and URL should all be aligned together. I would move it over to the right a couple pixels as well to match the spacing from the left-edge of the box to the left-edge of the favicon. Additionally, it should only be capturing the page title, not any text from the page itself (not sure if that's a page title in the mockup.) http://i.sevaan.com/image/3V1T3n0O0x03
Assignee | ||
Updated•9 years ago
|
Attachment #8605260 -
Flags: review?(standard8)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8605263 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8605260 -
Attachment is obsolete: true
Attachment #8605277 -
Flags: review?(standard8)
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8605274 -
Attachment is obsolete: true
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8605277 -
Attachment is obsolete: true
Attachment #8605277 -
Flags: review?(standard8)
Attachment #8605288 -
Flags: review?(standard8)
Comment 12•9 years ago
|
||
Comment on attachment 8605288 [details] [diff] [review] Patch v1.3: UX refresh for the available context area Review of attachment 8605288 [details] [diff] [review]: ----------------------------------------------------------------- One small nit, otherwise looks great. ::: browser/components/loop/content/js/roomViews.jsx @@ +312,5 @@ > // checkbox will be disabled in that case. > if (nextProps.editMode) { > this.props.mozLoop.getSelectedTabMetadata(function(metadata) { > var previewImage = metadata.favicon || ""; > + var description = metadata.title || metadata.description; Did you mean to make this change? If so, you forgot the roomViews.js update.
Attachment #8605288 -
Flags: review?(standard8) → review+
Assignee | ||
Comment 13•9 years ago
|
||
Patch for landing, carrying over r=Standard8.
Attachment #8605288 -
Attachment is obsolete: true
Attachment #8605800 -
Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c6ef413571dd
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Updated•9 years ago
|
QA Contact: bogdan.maris
Comment 16•9 years ago
|
||
Patch for aurora without the string removal. Approval Request Comment [Feature/regressing bug #]: Context in conversations for Hello [User impact if declined]: Improves the layout of the context details in the panel to make it more appealing. [Describe test coverage new/current, TreeHerder]: Has unit tests [Risks and why]: Minor, only affects loop specific code [String/UUID change made/needed]: None
Attachment #8606157 -
Flags: approval-mozilla-aurora?
Comment 17•9 years ago
|
||
UX looks fine, as shown in mockup, using latest Nightly, across platforms (Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5). Will mark this bug as verified fixed but keeping qe-verify+ to retest in Aurora.
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
status-firefox40:
--- → affected
Updated•9 years ago
|
Attachment #8606157 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 18•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/24b18a77678a
Flags: in-testsuite+
Comment 19•9 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #17) > UX looks fine, as shown in mockup, using latest Nightly, across platforms > (Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5). Will mark this > bug as verified fixed but keeping qe-verify+ to retest in Aurora. Also verified using latest Aurora across platforms (Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5).
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•