377401 - [Proto] Always 3 minimonths in Recurrence-dialog

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Berend Cornelius]

In the Recurrence dialog should initially appear 3 minimonths in the preview section

Comment 1

[Berend Cornelius]

Attachment: added two other minimonths to preview

Comment 2

[Michael Büttner (no reviews TFN)]

Comment on attachment 261472 [details] [diff] [review]
added two other minimonths to preview

>             <xul:row anonid="row">
>               <xul:recurrence-calendar anonid="minimonth"/>
>+               <xul:recurrence-calendar anonid="minimonth"/>
>+              <xul:recurrence-calendar anonid="minimonth"/>
>               <xul:spacer/>
>             </xul:row>
Please align the new elements with the recurrence-calendar above.

>+            <field name="mbStartUp">false</field>
What do you need this flag for?

>             <constructor>
>               <![CDATA[
>                 var self = this;
>                 this.mResizeHandler = function resizeHandler() { self.onResize(); };
>-                window.addEventListener("resize", this.mResizeHandler, true);
>+                window.addEventListener("resize", this.mResizeHandler, true); 
>+                this.onResize();
Is there any reason you explicitly calling onResize() here? The appropriate handler will be called anyway, isn't it so?

>-          var numHorizontal = (containerWidth-(containerWidth%contentWidth))/contentWidth;
>+	  var numHorizontal = 2
>+	  if (!this.mbStartUp){
>+          	numHorizontal = (containerWidth-(containerWidth%contentWidth))/contentWidth;
>+          }
>+	  else{
>+            this.mbStartUp = false;
>+	  }
I don't understand what mbStartUp stands for? The state of this flag is never changed...

>         <!-- recurrence pattern -->
>-        <xul:groupbox>
>+        <xul:groupbox class="recurrence-groupbox">
What is this new class for?

>-        <xul:groupbox>
>+        <xul:groupbox class="recurrence-groupbox">
Same question here.

>         <!-- preview -->
>-        <xul:groupbox flex="1">
>+        <xul:label class="recurrence-groupbox" value="&event.recurrence.preview.label;"/>
>+	<xul:recurrence-preview id="recurrence-preview" class="recurrence-groupbox" flex="1" /> 
>+<!--        <xul:groupbox flex="1">
>           <xul:caption label="&event.recurrence.preview.label;"/>
>           <xul:recurrence-preview id="recurrence-preview" flex="1"/>
>-        </xul:groupbox>
>+        </xul:groupbox>  -->
Please remove the comment here as it doesn't explain anything.

>-.datepicker-box {
>+datepicker-box {
> 	-moz-binding: url("chrome://calendar/content/sun-calendar-event-dialog-recurrence-datepicker.xml#datepicker-box");
> }
Why do you change the datepicker-box class? Is this relevant for this patch?

>-  -moz-padding-start: 8px;
>-  -moz-padding-end: 10px;
Why do you remove the padding here?

>-  -moz-padding-start: 8px;
>-  -moz-padding-end: 10px;
Same question here.

>-.minimonth-day[budy="true"] {
>+.minimonth-day[busy="true"] {
Nice catch...

Comment 3

[Berend Cornelius]

Attachment: second try...

Comment 4

[Michael Büttner (no reviews TFN)]

Comment on attachment 261777 [details] [diff] [review]
second try...

>-                window.addEventListener("resize", this.mResizeHandler, true);
>+                window.addEventListener("resize", this.mResizeHandler, true); 
just a style nit, there an additional space at the end of this line.

>-          <xul:grid style="min-width: 28em">
>+          <xul:grid >
again, just a style nit, there's an additional space before the '>'.

>-.minimonth-day[budy="true"] {
>+.minimonth-day[busy="true"] {
although this change is correct, it is not related to this bug. furthermore, it prevents this patch to go in before 0.5 has been shipped. please feel free to submit a new patch without this change, otherwise I'll check this in after 0.5 has been released.

r=mickey, i'll change the style nits before checking this in. thanks for the patch.

Comment 5

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 6

[Andreas Treumann]

verified with Lightning build 2007070304