Code review comment for lp:~pkunal-parmar/ubuntu-calendar-app/TimeLineView

Revision history for this message
Olivier Tilloy (osomon) wrote :

55 + property bool timeLineViewEnable : false

You can get rid of this extraneous property, and instead expose eventViewDelegate.source as an alias at the top level of EventView. Thus, instead of flipping the value of a boolean property, you’d simply set the source component directly. That’s less code, easier to read, and more flexible in case we want to add other types of views in the future.

85 + function loadSubDelegate() {
86 + source = eventView.timeLineViewEnable ? "TimeLineView.qml" : "DiaryView.qml"
87 + }
88 +
89 + Component.onCompleted: {
90 + loadSubDelegate();
91 + }

The above becomes useless if you get rid of timeLineViewEnable.

111 + onTimeLineViewEnableChanged :{
112 + loadSubDelegate();
113 + }

Same goes for the above.

review: Needs Fixing

« Back to merge proposal