Review Request: Allow user to select Calendar System to display in Calendar and Clock Applets

John Layt johnlayt at googlemail.com
Wed Oct 14 20:42:56 CEST 2009


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

(Updated 2009-10-14 18:42:56.059625)


Review request for Plasma.


Changes
-------

Clean-up of code based on Aarons comments, including merge of Aarons changes to simplify config dialog.  See responses to review comments for details, only one point outstanding.


Summary
-------

[Requires kdelibs revision 1013825]

Currently the Calendar and Clock applets can only display the global locale Calendar System, usually Gregorian.  However it would be useful for users to be able to display Calendar applets with different Calendar Systems, for example Hebrew or Hijri, similar in concept to being able to have multiple Clock applets showing different Timezones.  This change enables different applets to display different Calendar Systems and allows the user to configure each of these.  See my blog post at http://www.layt.net/john/blog/odysseus/displacement_activity for screen shots.

This is primarily achieved by moving most of the calendar configuration code from ClockApplet to CalendarTable from where it can be shared by any applet that embeds the CalendarTable.  The Calendar applet has been modified to use the common configuration, as has the base ClockApplet class which ensures all Clock applets do as well. [I've had some thoughts to move the generic code from CalendarTable to a new non-gui helper class so different calendar layout widgets could share it, but that's not needed yet].

The major implementation issue I have to raise is in the ClockApplet class where I now create the CalendarTable in the init() instead of in initExtenderExtender(), and the extender item now gets passed the already existing CalendarTable which I believe it then manages.  I _think_ this is safe to do as we will only ever have one calendar extender and it never appears to be deleted during the life of the applet, so I don't think the CalendarTable could be accidentally deleted.  This means there is always a calendarWidget and calendar object so the code can be simplified in lots of places to remove checks to see if it exists first.

This change does not yet address the issue in the Digital Clock applet that the displayed Date format is not correctly localised, but this involves a number of usability issues that I think are best addressed separately.

Changes:

ClockApplet:
  Remove all calendar configuration code and vars, call CalendarTable implementation instead
  Add convenience method calendar() for shortcut
  Clean-up clipboardMenu to only have localised date/time formats, add other calendar system options
  Remove unused method caseInsensitiveLessThan()

CalendarTable
  Add all calendar configuration code and vars from ClockApplet
  Add ability to set and save calendarType, special type of "locale" means use global locale type
  Improved validation

Calendar
  API to pass through calendar config calls to CalendarTable

generalConfig.ui
  Removed all calendar configuration options to new calendarConfig.ui

calendarConfig.ui
  New, has all calendar configuration options from generalConfig.ui
  Added combo box to select calendar system from.

Clock (digital applet)
  Display date on clock face in correct calendar system 

CalendarApplet
  Add d-> private class, use for variables
  Add new shared calendar config via CalendarTable
  Localise day number shown on Icon view

[My apologies if my over-zealous clean-up of code layout and renaming makes the diff a little hard to read in places]


Diffs (updated)
-----

  trunk/KDE/kdebase/workspace/libs/plasmaclock/CMakeLists.txt 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/calendar.h 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/calendar.cpp 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/calendarConfig.ui PRE-CREATION 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/calendartable.h 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/calendartable.cpp 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.h 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1035179 
  trunk/KDE/kdebase/workspace/libs/plasmaclock/generalConfig.ui 1035179 

Diff: http://reviewboard.kde.org/r/1378/diff


Testing
-------

Used plasmoidviewer to add/remove/play with lots of Calendar and Clock applets.


Thanks,

John



More information about the Plasma-devel mailing list