WIP
When finished, should fix T8225
TODO:
Remove the test data and integrate it with sinkDONE- Use the parent / children model in qml, instead of relying on the ".events" attribute (using DelegateModel I think)
cmollekopf |
WIP
When finished, should fix T8225
TODO:
Lint Skipped |
Unit Tests Skipped |
framework/src/domain/eventtreemodel.cpp | ||
---|---|---|
33 ↗ | (On Diff #31820) | This should probably be a live query, otherwise it won't updated. |
41 ↗ | (On Diff #31820) | You will have to react to rowsInserted/rowsRemoved as well. dataChanged only tells you about an individual index changing. |
43 ↗ | (On Diff #31820) | Synchronizing in the constructor is never a good idea, and synchronizing everything even less so. Synchronization should (almost) always be triggered by user interaction, see sinkfabric.cpp for examples. |
framework/src/domain/eventtreemodel.h | ||
2 ↗ | (On Diff #31820) | Your copyright is missing. |
57 ↗ | (On Diff #31820) | Maybe this should rather be a DayEventModel (or something alike?) The toplevel nodes are days after all, so that seems like a defining property of the model. |
In general we'll rather want a model that can display an unlimited number of days, not fixed 7.
The model should be configured with a date-filter, that we'd ideally forward to the sink query, but for now we'll just filter inside the model.
That date-filter will then define how many rows the model has.
That way we will be able to reuse the same model for a day-view, 5-day view, month-view ....
framework/src/domain/eventtreemodel.cpp | ||
---|---|---|
58 ↗ | (On Diff #31820) | You seem to assume that you only get events from a single week. Otherwise you will end up with e.g. all events that occur on a monday ever in the monday slot. |
65 ↗ | (On Diff #31820) | You have to notify about rowsInserted otherwise the view doesn't pick it up. |
Done:
Todo:
TODO:
Waiting on Michael to discuss the QML side
framework/src/domain/eventtreemodel.cpp | ||
---|---|---|
41 ↗ | (On Diff #31820) | I'm updating it to react to layoutChanged, rowsInserted / Moved / Removed as well, but from a quick look at sink's modelresult.cpp, the model doesn't seem to emit any of them, only dataChanged |
43 ↗ | (On Diff #31820) | Right, that was only a temporary thing to test my assumptions |
Great work, I think we can merge this after the nitpicks are fixed.
framework/src/domain/perioddayeventmodel.cpp | ||
---|---|---|
83 | const QDate & | |
218 | Perhaps convert this into: return {{1, "foo"}, {2, "bar"}}; | |
234 | Pass all non-pod types by const-reference. and then don't move instead. | |
257 | This std::move doesn't do anything AFAIK. int doesn't have a move constructor. | |
framework/src/domain/perioddayeventmodel.h | ||
114 | pass by const ref | |
121 | We use "const QDate &candidate" everywhere else, so let's stick to that. |
framework/src/domain/perioddayeventmodel.cpp | ||
---|---|---|
234 | I guess you're right. The really optimized version would be to write a setPeriodStart(const QDate &) plus a setPeriodStart(QDate &&) where move only happens in the second one. Plus, it also depends on how Qt pass the parameters from the view. I guess the C++98 default is the best in our case (simple and fairly optimized). More info here: https://youtu.be/xnqTKD8uD64?t=51m6s | |
257 | Right, I've just seen it. I think it's a leftover from when I wanted a setPeriodEnd instead of setPeriodLength |