658197 - "No messages match your search" message displaying badly when not en-US

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Jean-Bernard Marcon]

Attachment: adding < br/> to have more room for text under the image

When an initial search fails to have any result, a more extensive search is proposed hitting the ENTER key. If this second search also fails, the message
"No messages match your search" is displayed.
Problem is:
en-US string is diplayed ok for the div dimensions in which it is stuck along with the image. 
BUT for more verbose langs (see first screenshot with en-US / French / Dutch / Spanish) the string is badly cut.

This minor layout issue can be corrected with the suggested patch, just adding a < br/> in the .xhtml file
Of course you smart guys may consider to tweak the css file instead ;-)

Comment 1

[Jean-Bernard Marcon]

Attachment: capture showing layout problem

Comment 2

[Jean-Bernard Marcon]

Attachment: Capture showing French version after patch applied

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 533556 [details] [diff] [review]
adding < br/> to have more room for text under the image

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

Seems good.  I'ld prefer it if we could keep the line length under 80 characters, though.

r=me with that fixed.

And, while I'm here, you'll need a ui-review, too.  So, I like the way it looks, but I want the image to be centered over the text, not pushed over to the right.

ui-r=me with _that_ fixed.  :)

Thanks,
Blake.

Comment 4

[Jean-Bernard Marcon]

Attachment: adding < br/> to have more space left

this new tentative patch suggest a code indentation.

Comment 5

[Jean-Bernard Marcon]

Attachment: capture with patch applied - centered image/text highlighted

For the centered image request, I am not sure what you mean exactly. without css tweks in my patch, the image as shown in the screenshot is centered relatively to the text. Do you mean the image should be left-align ?

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 534214 [details]
capture with patch applied - centered image/text highlighted

Hmm.  That's what I want it to look like, but I see http://dl.dropbox.com/u/2301433/Screenshots/TooFarRight.png instead of the centered image.  I'm on Mac OS X, if that could make a difference.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 534212 [details] [diff] [review]
adding < br/> to have more space left

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

I mostly like this, but there are a couple of indentation/spacing nits that I think should be fixed.  (Also, I think it would be good to get the centering working on Mac, and Windows.  Now that I've tested it on both, I'm starting to think that it might be the English that's misaligned, instead of it being due to the platform.)

::: mail/base/content/glodaFacetView.xhtml
@@ +70,5 @@
>            <div id="data-column">
>              <div id="facet-date" class="facetious" type="date" />
>              <div class="results" id="results" type="message" />
> +            <div class="loading" id="showLoading">
> +					<span class="loading">

We use two-space indentation, and no tabs.

@@ +71,5 @@
>              <div id="facet-date" class="facetious" type="date" />
>              <div class="results" id="results" type="message" />
> +            <div class="loading" id="showLoading">
> +					<span class="loading">
> +						<img class="loading" 

There's an extra space at the end of this line.

@@ +72,5 @@
>              <div class="results" id="results" type="message" />
> +            <div class="loading" id="showLoading">
> +					<span class="loading">
> +						<img class="loading" 
> +							src="chrome://global/skin/icons/loading_16.png"/>

The "src=" should line up with the start of the "class=" here.

@@ +78,5 @@
> +					</span>
> +			</div>
> +            <div class="empty"	id="showEmpty">
> +					<span class="empty">
> +						<img class="empty" 

(And another extra space here.)

Comment 8

[Ludovic Hirlimann [:Usul]]

Attachment: Fix addressing the comments

Comment 9

[Ludovic Hirlimann [:Usul]]

Blake can we get this in and spawn the centered issue in another bug ?

Comment 10

[Blake Winton (:bwinton) (:☕️)]

I don't think so, mostly because I feel that it _really_ shouldn't be that hard to make it all line up.  (I took a stab at it yesterday, but ChromeBug doesn't work on Trunk, so I failed. :( )

Comment 11

[Blake Winton (:bwinton) (:☕️)]

(But ChromeBug works on trunk!  And it looks like removing:
padding: 1em;
from http://mxr.mozilla.org/comm-central/source/mail/base/content/glodaFacetView.css#814 fixes it, so r=me for the current patch with that added.)

Later,
Blake.

Comment 12

[Jean-Bernard Marcon]

Attachment: and yet another attempt (indentation and centering)

Comment 13

[Jean-Bernard Marcon]

Attachment: nd yet another attempt (indentation + centering)

Comment 14

[Jean-Bernard Marcon]

Comment on attachment 535355 [details] [diff] [review]
nd yet another attempt (indentation + centering)

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

Comment 15

[Ludovic Hirlimann [:Usul]]

Attachment: Cleanup up patch from goofy

Comment 16

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 535395 [details] [diff] [review]
Cleanup up patch from goofy

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

I like it.  I've got a couple of nits below, but would be happy to fix them when I land this.

Thanks,
Blake.

::: mail/base/content/glodaFacetView.xhtml
@@ +72,5 @@
>              <div class="results" id="results" type="message" />
> +            <div class="loading" id="showLoading">
> +              <span class="loading">
> +                <img class="loading"
> +                  src="chrome://global/skin/icons/loading_16.png"/>

src should line up with class.

@@ +73,5 @@
> +            <div class="loading" id="showLoading">
> +              <span class="loading">
> +                <img class="loading"
> +                  src="chrome://global/skin/icons/loading_16.png"/>
> +                  &glodaFacetView.loading.label;

&glodaFacetView should line up with <img.

@@ +80,5 @@
> +            <div class="empty"	id="showEmpty">
> +              <span class="empty">
> +                <img class="empty"
> +                     src="chrome://messenger/skin/icons/empty-search-results.png"/><br/>
> +                     &glodaFacetView.empty.label;

&glodaFacetView should line up with <img.

Comment 17

[Ludovic Hirlimann [:Usul]]

sorry :(

Comment 18

[Mark Banner (:standard8)]

Blake: can you fix this up and check it in? (or can someone post a new patch). I don't generally like checkin-needed for patches that need extra tweaks.

Comment 19

[Blake Winton (:bwinton) (:☕️)]

Committed as http://hg.mozilla.org/comm-central/rev/48d8cac97e59