Closed Bug 1081847 Opened 7 years ago Closed 7 years ago

Resize the conversation window to 300w x 272h

Categories

(Hello (Loop) :: Client, defect, P5)

defect
Points:
2

Tracking

(firefox39 verified)

VERIFIED FIXED
mozilla39
Iteration:
39.1 - 9 Mar
Tracking Status
firefox39 --- verified
Blocking Flags:
backlog backlog+

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [UX bug])

Attachments

(1 file, 2 obsolete files)

Following discussion on irc with Darrin, we should change the size of the conversation window to match the spec:

Standard8  darrin: hmm, http://people.mozilla.org/~mmaslaney/loop/Loop-spec.png has different dimensions to your mockups (300px width)
darrin     Standard8: what is the width in nightly?
Standard8  darrin: width: 260px, height 285px
darrin     Standard8: ya, looks like michael had 300w x 272h
darrin     which looks fine to me

This came out of a discussion about fitting the call url into the conversation window correctly.
This is a WIP patch. Needs visual testing on all platforms, and to check that minimizing the chat window still does the right thing.
backlog: --- → Fx36?
can QA validate if this patch works across platforms - see comment 1.
backlog: Fx36? → Fx37+
Keywords: qawanted
backlog: Fx37+ → Fx38?
Priority: -- → P5
Sevaan: This is something that Darrin had originally asked for when we were doing incoming calls from link clickers - so that the call url fitted in the window fully.

However, as call urls are going away, it isn't so necessary now. Unless we want to do it anyway for different UX/improved UX/some other reason?
Flags: needinfo?(sfranks)
Yes, let's do it. I'm finding things are starting to get cramped and the extra real estate will help make it feel roomier.
Flags: needinfo?(sfranks)
Keywords: qawanted
backlog: Fx38? → backlog+
Rank: 55
Flags: firefox-backlog+
Whiteboard: [UX bug]
(In reply to Mark Banner (:standard8) from comment #5)
> Sevaan: I've updated the patch and done some try builds, can you test them
> out and see what you think?

Looks good to me!
Flags: needinfo?(sfranks)
Sevaan's happy, so must be time for review.
Attachment #8504691 - Attachment is obsolete: true
Attachment #8568139 - Flags: review?(mdeboer)
Comment on attachment 8568139 [details] [diff] [review]
Resize the Loop conversation window to 300w x 272h.

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

This looks good to go, except you do need to add a check for the 'large' attribute at:

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/socialchat.xml#459

::: browser/base/content/browser.css
@@ +910,5 @@
>  
> +chatbox[large="true"] {
> +  width: 300px;
> +  heigth: 272px;
> +}

wouldn't it be nice if we could just:

```css
chatbox[width] {
  width: attr(width px);
}

chatbox[height] {
  height: attr(height px);
}
```

And in socialchat.xml, do:

```xml
<method name="getTotalChildWidth">
  <parameter name="aChatbox"/>
  <body><![CDATA[
    // These are from the CSS for the chatbox and must be kept in sync.
    // We can't use calcTotalWidthOf due to the transitions...
    const CHAT_WIDTH_OPEN = 260;
    const CHAT_WIDTH_MINIMIZED = 160;
    let openWidth = aChatbox.hasAttribute("width") ? parseInt(aChatbox.getAttribute("width", 10) : CHAT_WIDTH_OPEN;
    return aChatbox.minimized ? CHAT_WIDTH_MINIMIZED : openWidth;
  ]]></body>
</method>
```

But no, we'll need to wait until bug 435426 gets implemented.
Attachment #8568139 - Flags: review?(mdeboer) → review-
Updated patch to change the constant in social chat as well.
Attachment #8568139 - Attachment is obsolete: true
Attachment #8568478 - Flags: review?(mdeboer)
Comment on attachment 8568478 [details] [diff] [review]
Resize the Loop conversation window to 300w x 272h.

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

::: browser/base/content/socialchat.xml
@@ +465,4 @@
>            const CHAT_WIDTH_MINIMIZED = 160;
> +          if (aChatbox.hasAttribute("large")) {
> +            CHAT_WIDTH_OPEN = 300;
> +          }

What about keeping the const and do the following:

```js
let openWidth = aChatbox.hasAttribute("large") ? 300 : CHAT_WIDTH_OPEN;
return aChatbox.minimized ? CHAT_WIDTH_MINIMIZED : openWidth;
```

?
Attachment #8568478 - Flags: review?(mdeboer) → review+
Yeah, it looks better - I made that change before landing.

https://hg.mozilla.org/integration/fx-team/rev/10db494eb03c
Iteration: --- → 39.1 - 9 Mar
Points: --- → 2
Flags: qe-verify+
Target Milestone: --- → mozilla39
Blocks: 1141509
No longer blocks: 1141509
Depends on: 1141509
Verified that the default size of the conversation window is 300px and height is 285px (as per bug 1141509 comment 3) across platforms (Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit) using latest Aurora and Nightly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.