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

Revision history for this message
Kunal Parmar (pkunal-parmar) 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.

Some how I am not able to expose eventViewDelegate.source property under EventView scope, it gives me error "Invalid alias reference. Unable to find id "eventViewDelegate".

I instead created new property under EventView "property string eventViewType", and binded it to eventViewDelegate "source: eventView.eventViewType"

« Back to merge proposal