'Re: Review Request 111413: Plan - Set First Day of Week to Monday if using ISO Week for week numberi'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       calligra-devel
Subject:    Re: Review Request 111413: Plan - Set First Day of Week to Monday if using ISO Week for week numberi
From:       "Alvaro Soliverez" <asoliverez () kde ! org>
Date:       2013-11-24 18:13:13
Message-ID: 20131124181313.14618.54105 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111413/
-----------------------------------------------------------

(Updated Nov. 24, 2013, 6:13 p.m.)

Review request for Calligra.

Changes
-------

Split the patch for kdgantt and fixed the remaining issues

Bugs: 321290
    http://bugs.kde.org/show_bug.cgi?id=321290

Repository: calligra

Description
-------

Plan uses week numbering extensively. The default week numbering in KDE is ISO Week, \
which uses Monday as first day of the week, regardless of other locale settings.

To make it consistent through the application, if week numbering format is ISO Week, \
set the first day of the week to Monday, and display it accordingly in charts and \
graphs.

Without this patch, the week could be set to start on Sunday, and week would be shown \
on graphs as ranging from Sunday to Saturday, but the week number of working days \
would be off by one.

Example: 

- Week numbering: ISO Week
- First day of week: Sunday

Without patch:
- Week displayed: 2013-07-07 to 2013-07-13
- Week number shown on charts: 27

- Expected week number: 28 (Week 27 ends on Sunday 2013-07-07)

With patch:
- Week displayed: 2013-07-08 to 2013-07-14
- Week number shown on charts: 28

Additionally, there were a few calls to QDate::week() to get the week number, instead \
of using KGlobalCalendar.

Diffs (updated)
-----

  plan/libs/models/kcalendar/kdatetable.cpp b389345 
  plan/libs/models/kptaccountsmodel.cpp 71c3100 
  plan/libs/ui/kptganttview.cpp 81073a3 

Diff: http://git.reviewboard.kde.org/r/111413/diff/

Testing
-------

Tested on a live file. It displays calendars and gantt charts correctly.

Thanks,

Alvaro Soliverez

[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/111413/">http://git.reviewboard.kde.org/r/111413/</a>
  </td>
    </tr>
   </table>
   <br />

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for Calligra.</div>
<div>By Alvaro Soliverez.</div>

<p style="color: grey;"><i>Updated Nov. 24, 2013, 6:13 p.m.</i></p>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Split the patch for kdgantt and fixed the remaining issues</pre>  </td>
 </tr>
</table>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>

 <a href="http://bugs.kde.org/show_bug.cgi?id=321290">321290</a>

</div>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Plan uses week numbering extensively. The default week numbering in KDE \
is ISO Week, which uses Monday as first day of the week, regardless of other locale \
settings.

To make it consistent through the application, if week numbering format is ISO Week, \
set the first day of the week to Monday, and display it accordingly in charts and \
graphs.

Without this patch, the week could be set to start on Sunday, and week would be shown \
on graphs as ranging from Sunday to Saturday, but the week number of working days \
would be off by one.

Example: 

- Week numbering: ISO Week
- First day of week: Sunday

Without patch:
- Week displayed: 2013-07-07 to 2013-07-13
- Week number shown on charts: 27

- Expected week number: 28 (Week 27 ends on Sunday 2013-07-07)

With patch:
- Week displayed: 2013-07-08 to 2013-07-14
- Week number shown on charts: 28

Additionally, there were a few calls to QDate::week() to get the week number, instead \
of using KGlobalCalendar.

</pre>
  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Tested on a live file. It displays calendars and gantt charts \
correctly.</pre>  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> \
(updated)</h1> <ul style="margin-left: 3em; padding-left: 0;">

 <li>plan/libs/models/kcalendar/kdatetable.cpp <span style="color: \
grey">(b389345)</span></li>

 <li>plan/libs/models/kptaccountsmodel.cpp <span style="color: \
grey">(71c3100)</span></li>

 <li>plan/libs/ui/kptganttview.cpp <span style="color: grey">(81073a3)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/111413/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

[prev in list] [next in list] [prev in thread] [next in thread] 