D12438: Fix PlasmaCalendar widget to not mark days with minor events

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
kossebau marked an inline comment as done.
Closed by commit R242:67f5ad29e8e6: Fix PlasmaCalendar widget to not mark days 
with minor events (authored by kossebau).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12438?vs=32782=33501

REVISION DETAIL
  https://phabricator.kde.org/D12438

AFFECTED FILES
  src/declarativeimports/calendar/daysmodel.cpp
  src/declarativeimports/calendar/daysmodel.h
  src/declarativeimports/calendar/qml/DayDelegate.qml

To: kossebau, #plasma, broulik, davidedmundson
Cc: davidedmundson, broulik, #frameworks, michaelh, bruns

D12438: Fix PlasmaCalendar widget to not mark days with minor events

[Friedrich W . H . Kossebau]

kossebau marked an inline comment as done.
kossebau added inline comments.

INLINE COMMENTS

> broulik wrote in daysmodel.cpp:223
> `std::find_if`

Tried, but seems there is no simple way to use std::find_if or std::any_of 
algorithms here for the usecase of checking all values for a given key in a 
QMultiHash. The algorithms all only take fixed first and last iterators, so one 
first has to estimate the last iterator by something like std::equal_range, 
which as result means iterating 2x over the entries and does not make the code 
more comprehensible to the reader.
So as discussed on irc staying with current code for now.

> davidedmundson wrote in DayDelegate.qml:105
> surely this should be model !== undefined && model.contains
> 
> otherwise we're doing a filter twice.

Haven't got the complete picture on this code, the purpose of 
18af8703039390de8b803a7a9b3c5cfe48f70b7a 
 
leaves me slightly puzzled, so better to be done separately, cannot yet oversee 
the potential side effects.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fixdisplayofminorevents

REVISION DETAIL
  https://phabricator.kde.org/D12438

To: kossebau, #plasma, broulik, davidedmundson
Cc: davidedmundson, broulik, #frameworks, michaelh, bruns

D12438: Fix PlasmaCalendar widget to not mark days with minor events

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added inline comments.

INLINE COMMENTS

> DayDelegate.qml:105
>  Loader {
> -active: model.containsEventItems !== undefined && 
> model.containsEventItems
> +active: model.containsMajorEventItems !== undefined && 
> model.containsMajorEventItems
>  anchors.bottom: parent.bottom

surely this should be model !== undefined && model.contains

otherwise we're doing a filter twice.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fixdisplayofminorevents

REVISION DETAIL
  https://phabricator.kde.org/D12438

To: kossebau, #plasma, broulik, davidedmundson
Cc: davidedmundson, broulik, #frameworks, michaelh, bruns

D12438: Fix PlasmaCalendar widget to not mark days with minor events

[Kai Uwe Broulik]

broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  address comment then ship it

INLINE COMMENTS

> daysmodel.cpp:223
> +auto it = m_eventsData.find(date);
> +while (it != m_eventsData.end() && it.key() == date) {
> +if (!it.value().isMinor()) {

`std::find_if`

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fixdisplayofminorevents

REVISION DETAIL
  https://phabricator.kde.org/D12438

To: kossebau, #plasma, broulik
Cc: broulik, #frameworks, michaelh, bruns

D12438: Fix PlasmaCalendar widget to not mark days with minor events

[Friedrich W . H . Kossebau]

kossebau created this revision.
kossebau added a reviewer: Plasma.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
kossebau requested review of this revision.

REVISION SUMMARY
  CalendarEvents::EventData (in KDeclarative) has a property isMinor, which is
  described as "If set to true, it won't be marked in the calendar grid".
  
  This patch fulfils that promise, by introducing separate roles on the model
  which explicitly report the existance of any major or any minor events at a
  given day, so that the marker loader can concentrate on the major events.

TEST PLAN
  Using a plasmacalendar plugin which feeds events with isMinor=true the
  widget now only marks days which have major events.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fixdisplayofminorevents

REVISION DETAIL
  https://phabricator.kde.org/D12438

AFFECTED FILES
  src/declarativeimports/calendar/daysmodel.cpp
  src/declarativeimports/calendar/daysmodel.h
  src/declarativeimports/calendar/qml/DayDelegate.qml

To: kossebau, #plasma
Cc: #frameworks, michaelh, bruns