Closed Bug 1142587 Opened 7 years ago Closed 7 years ago

Implement indicators for context in conversations in the panel alongside room names

Categories

(Hello (Loop) :: Client, enhancement, P1)

enhancement
Points:
2

Tracking

(firefox40 verified)

VERIFIED FIXED
mozilla40
Iteration:
40.2 - 27 Apr
Tracking Status
firefox40 --- verified

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [context])

User Story

As a desktop client user, I want my Hello panel to include indications about attached context so that I know the context of conversations before joining them.

Acceptance criteria:
* The favicon of the URL context added to conversations appears next to the conversation name in the panel
* Hovering over the Favicon displays a tooltip with the site title
* Conversations where no URL context was added get displayed without changes

Attachments

(1 file, 1 obsolete file)

Implementation bug for implementing the indicators for context for conversations
Blocks: 1130079
No longer blocks: 1115340
No longer depends on: 1130079
Rank: 23
Flags: qe-verify+
Flags: firefox-backlog+
Rank: 23 → 7
Priority: P2 → P1
I have a patch almost ready for this.

Sevaan: one question, should we open the url in a tab when you click on the context indicator?
Assignee: nobody → standard8
Iteration: --- → 40.2 - 27 Apr
Points: --- → 2
(In reply to Mark Banner (:standard8) from comment #1)
> Sevaan: one question, should we open the url in a tab when you click on the
> context indicator?
Flags: needinfo?(sfranks)
Basically complete, depends on patch in bug 1154708. Needs answer to UX question.
(In reply to Mark Banner (:standard8) from comment #2)
> (In reply to Mark Banner (:standard8) from comment #1)
> > Sevaan: one question, should we open the url in a tab when you click on the
> > context indicator?

Yes please. A new tab, actually.
Flags: needinfo?(sfranks)
This finishes the new indicator and makes it clickable for opening the link.

I've assumed 16x16 for the icon as we seem to be heading towards favicons.
Attachment #8592935 - Attachment is obsolete: true
Attachment #8595336 - Flags: review?(mdeboer)
Comment on attachment 8595336 [details] [diff] [review]
Implement indicators for context in conversations in the panel alongside room names.

Review of attachment 8595336 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/loop/content/css/panel.css
@@ +365,5 @@
> +}
> +
> +.room-entry-context-item > a > img {
> +  height: 16px;
> +  width: 16px;

I *think* that if you specify only the width here, it'll resize the source img maintaining aspect ratio.

::: browser/components/loop/content/js/panel.jsx
@@ +443,5 @@
> +      this.props.mozLoop.openURL(event.currentTarget.href);
> +    },
> +
> +    render: function() {
> +      if (!this.props.roomUrls || !this.props.roomUrls.length ||

I vote for declaring a shorthand:

```js
var roomUrl = this.props.roomUrls && this.props.roomUrls[0];
if (!roomUrl) {
  return null;
}
...
```

@@ +444,5 @@
> +    },
> +
> +    render: function() {
> +      if (!this.props.roomUrls || !this.props.roomUrls.length ||
> +          !this.props.roomUrls[0].location) {

How can this last one ever be the case?
Attachment #8595336 - Flags: review?(mdeboer) → review+
(In reply to Mike de Boer [:mikedeboer] from comment #6)
> > +.room-entry-context-item > a > img {
> > +  height: 16px;
> > +  width: 16px;
> 
> I *think* that if you specify only the width here, it'll resize the source
> img maintaining aspect ratio.

Right, but I don't want to do that. Given the expected switch to favicons, I think keeping it square is fine and I'd rather be explicit here. If we change, then we can come back and revisit.
Target Milestone: --- → mozilla40
https://hg.mozilla.org/mozilla-central/rev/7be3f9bb88bf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
QA Contact: bogdan.maris
Verified that the indicators for context are properly displayed in panel alongside room names, user story checks with the implementation. Tested using latest Aurora on Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.