Merge lp:~gang65/ubuntu-calendar-app/ubuntu-calendar-app-quick-add-event into lp:ubuntu-calendar-app

Proposed by Bartosz Kosiorek
Status: Merged
Approved by: Kunal Parmar
Approved revision: 234
Merged at revision: 252
Proposed branch: lp:~gang65/ubuntu-calendar-app/ubuntu-calendar-app-quick-add-event
Merge into: lp:ubuntu-calendar-app
Diff against target: 411 lines (+94/-60)
7 files modified
MonthComponent.qml (+24/-6)
MonthView.qml (+16/-15)
TimeLineBackground.qml (+5/-4)
TimeLineBase.qml (+19/-5)
TimeLineBaseComponent.qml (+9/-8)
YearView.qml (+18/-20)
debian/copyright (+3/-2)
To merge this branch: bzr merge lp:~gang65/ubuntu-calendar-app/ubuntu-calendar-app-quick-add-event
Reviewer Review Type Date Requested Status
Kunal Parmar Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Ubuntu Calendar Developers Pending
Review via email: mp+215067@code.launchpad.net

Commit message

Implementation of quick add events by long pressing, for Day, Week, Month and Year view.

Description of the change

Implementation of quick add events by long pressing, for Day, Week, Month and Year view.

To post a comment you must log in.
225. By Bartosz Kosiorek

Rebase/merge from trunk revision 238

226. By Bartosz Kosiorek

Fix reference error

227. By Bartosz Kosiorek

Implementation of the quick add for Day and Week view

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

24 + if (tabs.selectedTabIndex == 1) {

I think, this is breaking encapsulation, I mean MonthComponenent is not supposed to know about tabs and its selected index.

I think its much better to add some property in monthcomponent like type or something, and use that to identify if month is inside yearview or monthview. and use that instead to make decision what signal to emit.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

23 + //If monthView is clicked then open selected DayView
24 + if (tabs.selectedTabIndex == 1) {
25 + root.dateSelected(new Date(intern.monthStartYear,
26 + intern.monthStartMonth,
27 + intern.monthStartDate+index,0,0,0,0));
28 + }
29 + //If yearView is clicked then open selected MonthView
30 + else if (tabs.selectedTabIndex == 0) {
31 + yearViewPage.monthSelected(new Date(intern.monthStartYear,
32 + intern.monthStartMonth,
33 + intern.monthStartDate+index,0,0,0,0));
34 + }

date is same in both condition, may be we should move it out of condition ?

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

100 delegate: TimeLineBase {
101 @@ -78,7 +75,7 @@
...
110 @@ -87,6 +84,9 @@
111 delegate: comp
113
114 + TimeLineBackground {
115 + }
116 +

I think, rather than handling mouse event inside TimeLineBackground, its better to handle that inside TimeLineBase.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

17 + pageStack.push(Qt.resolvedUrl("NewEvent.qml"), {"date":selectedDate});

we also need to pass model as property now.

review: Needs Fixing
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

> 100 delegate: TimeLineBase {
> 101 @@ -78,7 +75,7 @@
> ...
> 110 @@ -87,6 +84,9 @@
> 111 delegate: comp
> 113
> 114 + TimeLineBackground {
> 115 + }
> 116 +
>
> I think, rather than handling mouse event inside TimeLineBackground, its
> better to handle that inside TimeLineBase.
The problem is that in TimeLineBase I do not have information about hour. I needs that information to set event for specific hour.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > I think, rather than handling mouse event inside TimeLineBackground, its
> > better to handle that inside TimeLineBase.
> The problem is that in TimeLineBase I do not have information about hour. I
> needs that information to set event for specific hour.

Height of timeline base is equal to units.gu(10) * 24, specified in TimeLineBackground.qml,
so if you get y location of click and divide it with units.gu(10),
I think you should be able to get hour and use that.

228. By Bartosz Kosiorek

Fix remarks after review

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hello Kunal.

Could you please take a look at the latest version?

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
229. By Bartosz Kosiorek

Display hours only at the middle of the week

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Text conflict in YearView.qml
1 conflicts encountered.
bzr: ERROR: Conflicts from merge

Hi, you need to merge code from trunk.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

77 + MouseArea {
78 + anchors.fill: parent
79 + onPressAndHold: {
80 + var selectedDate = new Date(day);
81 + selectedDate.setHours(index)
82 + pageStack.push(Qt.resolvedUrl("NewEvent.qml"), {"date":selectedDate, "model":eventModel});
83 + }
84 + }

And this still need to be fixed. We should move code to timeline base.

Height of timeline base is equal to units.gu(10) * 24, specified in TimeLineBaseComponent.qml,
so if you get y location of click and divide it with units.gu(10),
I think you should be able to get hour and use that.

review: Needs Fixing
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hello Kunal.

Unfortunately I have problems with moving MouseArea into TimeLineBaseComponent.
I spend on that issue a lot of hours, unfortunately without success.

Could you please try to prepare some example implementation of such moving?

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Hi, Sure, I will provide sample.

But can you resolve the merge conflicts from YearView.qml

188 +<<<<<<< TREE
189 Page{
190 id: root
191
192 +=======
193 +
194 +PathViewBase {
195 + id: yearViewPage
196 + objectName: "YearView"
197 +>>>>>>> MERGE-SOURCE

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hi Kunal.

I changed Year View Page from "id: root" to "id: yearViewPage" to be able to run:

 + //If yearView is clicked then open selected MonthView
48 + else {
49 + yearViewPage.monthSelected(selectedDate);
50 + }

When this conflict appears for you?
I do not see any conflicts.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

>
> When this conflict appears for you?
> I do not see any conflicts.

Hi, I see conflicts in diff displayed in this MR.
looks like you have not merged trunk code back to your branch.
That is also then reason, jenkins testing is failing.

230. By Bartosz Kosiorek

Sync with lp:ubuntu-calendar-app trunk

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hi Kunal.

I already resolved the conflicts. You could take a look at the code...

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> Hi Kunal.
>
> I already resolved the conflicts. You could take a look at the code...

Hi, thanks, I see no conflicts now. I am also trying how to move code from TimeLineBackground to TimeLineBase

It strage, as you said, Its not getting mouse events. I will look more. Pls give me some time

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Hello,

I found the issue. Issue was that TimeLineBase was deleting its children before recreating the event bubbles.
So MouseArea was also getting deleted.

So to be able to detect mouse event in TimeLinebase, Please modify destroyAllChildren() as below.

    function destroyAllChildren() {
        for( var i = children.length - 1; i >= 0; --i ) {
            if( children[i].objectName === "mouseArea" ){
                continue;
            }
            children[i].visible = false;
            if( children[i].objectName !== "separator") {
                children[i].destroy();
            }
        }
    }

And add MouseArea in TimeLineBase as shown below

Item {
    id: bubbleOverLay

    ...

    MouseArea {
        anchors.fill: bubbleOverLay
        objectName: "mouseArea"
        onClicked: {
            var selectedDate = new Date(day);
            var index = parseInt(mouseY / hourHeight);
            selectedDate.setHours(index)
            print(mouseY + " ####### Index:"+ index +",,,,"+ selectedDate);
        }
    }

Thanks for your support and patient

231. By Bartosz Kosiorek

Move MouseArea into TimeLineBase

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hi Kunal.

I moved MouseArea into TimeLineBase, tested it, and it is working perfectly for me.

Thank you for your help. Your tips were very useful.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

It looks good,

Just one minor comment, rather then accessing yearViewPage directly, we should introduce new signal and yerview should handle that signal

57 + else {
58 + yearViewPage.monthSelected(selectedDate);
59 + }

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

71 + isYearView: false;
72

Dont we need to define isYearView: true in YearView page ?

232. By Bartosz Kosiorek

Fix indentation

233. By Bartosz Kosiorek

Sync with main

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
234. By Bartosz Kosiorek

Use signals instead of direct call method from YearView

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Hi Kunal.
Your remarks was implemented. Branch is ready to push.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Hi, there are lot of formatting and re-factoring changes mixed with new feature implementation. I would appreciate if you dont mix those.
As its little hard to review changes, as not sure what can cause regression and what not.

Anyway, I will review this changes, no need to revert anything.
Thanks

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'MonthComponent.qml'
2--- MonthComponent.qml 2014-04-12 05:35:56 +0000
3+++ MonthComponent.qml 2014-04-19 21:47:33 +0000
4@@ -10,6 +10,7 @@
5 property bool showEvents: false
6
7 property var currentMonth;
8+ property var isYearView;
9
10 property string dayLabelFontSize: "medium"
11 property string dateLabelFontSize: "large"
12@@ -19,6 +20,7 @@
13 property alias dayLabelDelegate : dayLabelRepeater.delegate
14 property alias dateLabelDelegate : dateLabelRepeater.delegate
15
16+ signal monthSelected(var date);
17 signal dateSelected(var date)
18
19 height: ubuntuShape.height
20@@ -188,7 +190,7 @@
21 sourceComponent: isToday && isCurrentMonth ? highLightComp : undefined
22 }
23
24- Label{
25+ Label {
26 id: dateLabel
27 anchors.centerIn: parent
28 width: parent.width
29@@ -208,7 +210,7 @@
30 }
31 }
32
33- Rectangle{
34+ Rectangle {
35 width: units.gu(1)
36 height: width
37 radius: height/2
38@@ -221,12 +223,28 @@
39 anchors.horizontalCenter: dateLabel.horizontalCenter
40 }
41
42- MouseArea{
43+ MouseArea {
44 anchors.fill: parent
45+ onPressAndHold: {
46+ var selectedDate = new Date();
47+ selectedDate.setFullYear(intern.monthStartYear)
48+ selectedDate.setMonth(intern.monthStartMonth + 1)
49+ selectedDate.setDate(date)
50+ selectedDate.setMinutes(60, 0, 0)
51+ pageStack.push(Qt.resolvedUrl("NewEvent.qml"), {"date":selectedDate, "model":eventModel});
52+ }
53 onClicked: {
54- root.dateSelected(new Date(intern.monthStartYear,
55- intern.monthStartMonth,
56- intern.monthStartDate+index,0,0,0,0));
57+ var selectedDate = new Date(intern.monthStartYear,
58+ intern.monthStartMonth,
59+ intern.monthStartDate + index, 0, 0, 0, 0)
60+ //If monthView is clicked then open selected DayView
61+ if ( isYearView === false ) {
62+ root.dateSelected(selectedDate);
63+ }
64+ //If yearView is clicked then open selected MonthView
65+ else {
66+ root.monthSelected(selectedDate);
67+ }
68 }
69 }
70 }
71
72=== modified file 'MonthView.qml'
73--- MonthView.qml 2014-04-12 05:34:13 +0000
74+++ MonthView.qml 2014-04-19 21:47:33 +0000
75@@ -32,18 +32,18 @@
76 }
77
78 function nextMonth() {
79- currentMonth = addMonth(currentMonth,1);
80- }
81-
82- function previousMonth(){
83- currentMonth = addMonth(currentMonth,-1);
84- }
85-
86- function addMonth(date,month){
87- return new Date(date.getFullYear(),date.getMonth()+month,1,0,0,0);
88- }
89-
90- delegate: MonthComponent{
91+ currentMonth = addMonth(currentMonth, 1);
92+ }
93+
94+ function previousMonth() {
95+ currentMonth = addMonth(currentMonth, -1);
96+ }
97+
98+ function addMonth(date,month) {
99+ return new Date(date.getFullYear(), date.getMonth() + month, 1, 0, 0, 0);
100+ }
101+
102+ delegate: MonthComponent {
103 property bool isCurrentItem: index === monthViewPath.currentIndex
104
105 showEvents: true
106@@ -52,15 +52,16 @@
107 height: parent.height - units.gu(5)
108
109 currentMonth: getMonthDate();
110+ isYearView: false
111
112 function getMonthDate() {
113 switch( monthViewPath.indexType(index)) {
114 case 0:
115- return monthViewPath.addMonth(monthViewPath.startMonth,0);
116+ return monthViewPath.addMonth(monthViewPath.startMonth, 0);
117 case -1:
118- return monthViewPath.addMonth(monthViewPath.startMonth,-1);
119+ return monthViewPath.addMonth(monthViewPath.startMonth, -1);
120 case 1:
121- return monthViewPath.addMonth(monthViewPath.startMonth,1);
122+ return monthViewPath.addMonth(monthViewPath.startMonth, 1);
123 }
124 }
125
126
127=== modified file 'TimeLineBackground.qml'
128--- TimeLineBackground.qml 2013-09-04 13:37:10 +0000
129+++ TimeLineBackground.qml 2014-04-19 21:47:33 +0000
130@@ -3,20 +3,21 @@
131
132 Column {
133 width: parent.width
134- Repeater{
135+ Repeater {
136 model: 24 // hour in a day
137
138 delegate: Rectangle {
139 width: parent.width
140 height: units.gu(10)
141- color: ( index % 2 == 0) ? "#4c875b" : "#86c07f"
142- Label{
143+ color: (index % 2 == 0) ? "#4c875b" : "#86c07f"
144+
145+ Label {
146 id: timeLabel
147
148 // TRANSLATORS: this is a time formatting string,
149 // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions
150 text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("hh ap"))
151- color:"white"
152+ color: "white"
153 anchors.horizontalCenter: parent.horizontalCenter
154 anchors.verticalCenter: parent.verticalCenter
155 fontSize: "x-large"
156
157=== modified file 'TimeLineBase.qml'
158--- TimeLineBase.qml 2014-04-05 05:00:55 +0000
159+++ TimeLineBase.qml 2014-04-19 21:47:33 +0000
160@@ -10,8 +10,19 @@
161 property int hourHeight: units.gu(10)
162
163 property var model;
164+
165+ MouseArea {
166+ anchors.fill: parent
167+ objectName: "mouseArea"
168+ onPressAndHold: {
169+ var selectedDate = new Date(day);
170+ var hour = parseInt(mouseY / hourHeight);
171+ selectedDate.setHours(hour)
172+ pageStack.push(Qt.resolvedUrl("NewEvent.qml"), {"date":selectedDate, "model":eventModel});
173+ }
174+ }
175
176- TimeSeparator{
177+ TimeSeparator {
178 id: separator
179 objectName: "separator"
180 width: bubbleOverLay.width
181@@ -25,7 +36,7 @@
182 }
183
184 function showEventDetails(event) {
185- pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event,"model":model});
186+ pageStack.push(Qt.resolvedUrl("EventDetails.qml"), {"event":event,"model":model});
187 }
188
189 function createEvents() {
190@@ -38,10 +49,10 @@
191 var endDate = new Date(day).endOfDay();
192
193 var items = model.getItems(startDate,endDate);
194- for(var i = 0 ; i < items.length ; ++i) {
195+ for(var i = 0; i < items.length; ++i) {
196 var event = items[i];
197 if(event.allDay === false) {
198- bubbleOverLay.createEvent(event,event.startDateTime.getHours());
199+ bubbleOverLay.createEvent(event, event.startDateTime.getHours());
200 }
201 }
202
203@@ -52,6 +63,9 @@
204
205 function destroyAllChildren() {
206 for( var i = children.length - 1; i >= 0; --i ) {
207+ if( children[i].objectName === "mouseArea" ) {
208+ continue;
209+ }
210 children[i].visible = false;
211 if( children[i].objectName !== "separator") {
212 children[i].destroy();
213@@ -59,7 +73,7 @@
214 }
215 }
216
217- function createEvent( event ,hour) {
218+ function createEvent(event, hour) {
219 var eventBubble = delegate.createObject(bubbleOverLay);
220
221 eventBubble.clicked.connect( bubbleOverLay.showEventDetails );
222
223=== modified file 'TimeLineBaseComponent.qml'
224--- TimeLineBaseComponent.qml 2014-04-05 08:53:53 +0000
225+++ TimeLineBaseComponent.qml 2014-04-19 21:47:33 +0000
226@@ -70,7 +70,7 @@
227 width: parent.width
228 height: parent.height
229
230- AllDayEventComponent{
231+ AllDayEventComponent {
232 id: allDayContainer
233 type: root.type
234 startDay: root.startDay
235@@ -80,7 +80,7 @@
236 }
237 }
238
239- Flickable{
240+ Flickable {
241 id: timeLineView
242
243 width: parent.width
244@@ -91,16 +91,16 @@
245
246 clip: true
247
248- TimeLineBackground{
249+ TimeLineBackground {
250 }
251
252- Row{
253+ Row {
254 id: week
255 width: parent.width
256 height: parent.height
257 anchors.top: parent.top
258
259- Repeater{
260+ Repeater {
261 model: type == ViewType.ViewTypeWeek ? 7 : 1
262
263 delegate: TimeLineBase {
264@@ -108,11 +108,12 @@
265 anchors.top: parent.top
266 width: {
267 if( type == ViewType.ViewTypeWeek ) {
268- parent.width/7
269+ parent.width / 7
270 } else {
271 (parent.width)
272 }
273 }
274+
275 height: parent.height
276 delegate: comp
277 day: startDay.addDays(index)
278@@ -127,9 +128,9 @@
279 }
280 }
281
282- Component{
283+ Component {
284 id: comp
285- EventBubble{
286+ EventBubble {
287 type: {
288 if( root.type == ViewType.ViewTypeWeek ) {
289 narrowType
290
291=== modified file 'YearView.qml'
292--- YearView.qml 2014-04-12 05:34:13 +0000
293+++ YearView.qml 2014-04-19 21:47:33 +0000
294@@ -2,8 +2,8 @@
295 import Ubuntu.Components 0.1
296
297 import "dateExt.js" as DateExt
298-Page{
299- id: root
300+Page {
301+ id: yearViewPage
302
303 property int currentYear: DateExt.today().getFullYear();
304 signal monthSelected(var date);
305@@ -31,7 +31,7 @@
306
307 property int scrollMonth: 0;
308 property bool isCurrentItem: index == pathView.currentIndex
309- property int year: (root.currentYear + pathView.indexType(index))
310+ property int year: (yearViewPage.currentYear + pathView.indexType(index))
311
312 width: parent.width
313 height: parent.height
314@@ -45,35 +45,35 @@
315
316 model: 12 /* months in a year */
317
318- onYearChanged : {
319- scrollMonth=0;
320- yearView.positionViewAtIndex(scrollMonth,GridView.Beginning);
321+ onYearChanged: {
322+ scrollMonth = 0;
323+ yearView.positionViewAtIndex(scrollMonth, GridView.Beginning);
324 }
325
326 //scroll in case content height changed
327 onHeightChanged: {
328- scrollMonth=0;
329- yearView.positionViewAtIndex(scrollMonth,GridView.Beginning);
330+ scrollMonth = 0;
331+ yearView.positionViewAtIndex(scrollMonth, GridView.Beginning);
332 }
333
334
335 Connections{
336 target: pathView
337- onScrollUp:{
338+ onScrollUp: {
339 scrollMonth -= 2;
340 if(scrollMonth < 0) {
341 scrollMonth = 0;
342 }
343- yearView.positionViewAtIndex(scrollMonth,GridView.Beginning);
344+ yearView.positionViewAtIndex(scrollMonth, GridView.Beginning);
345 }
346
347- onScrollDown:{
348+ onScrollDown: {
349 scrollMonth += 2;
350 var visibleMonths = yearView.height / cellHeight;
351 if( scrollMonth >= (11 - visibleMonths)) {
352 scrollMonth = (11 - visibleMonths);
353 }
354- yearView.positionViewAtIndex(scrollMonth,GridView.Beginning);
355+ yearView.positionViewAtIndex(scrollMonth, GridView.Beginning);
356 }
357 }
358
359@@ -81,11 +81,12 @@
360 width: yearView.cellWidth
361 height: yearView.cellHeight
362
363- MonthComponent{
364+ MonthComponent {
365 id: monthComponent
366 showEvents: false
367- currentMonth: new Date(yearView.year,index,1,0,0,0,0)
368-
369+ currentMonth: new Date(yearView.year, index, 1, 0, 0, 0, 0)
370+
371+ isYearView: true
372 anchors.fill: parent
373 anchors.margins: units.gu(0.5)
374
375@@ -94,11 +95,8 @@
376 monthLabelFontSize: "medium"
377 yearLabelFontSize: "small"
378
379- MouseArea{
380- anchors.fill: parent
381- onClicked: {
382- root.monthSelected(monthComponent.currentMonth);
383- }
384+ onMonthSelected: {
385+ yearViewPage.monthSelected(date);
386 }
387 }
388 }
389
390=== modified file 'debian/copyright'
391--- debian/copyright 2013-07-24 21:24:52 +0000
392+++ debian/copyright 2014-04-19 21:47:33 +0000
393@@ -3,7 +3,8 @@
394 Source: https://launchpad.net/ubuntu-calendar-app
395
396 Files: *
397-Copyright: 2013 Canonical Ltd.
398+Copyright: 2014 Canonical Ltd.
399+ 2014 Bartosz Kosiorek <gang65@poczta.onet.pl>
400 2013 Carla Sella <carla.sella@gmail.com>
401 2013 Frank Mertens <frank@cyblogic.de>
402 2013 Kunal Parmar <pkunal.parmar@gmail.com>
403@@ -13,7 +14,7 @@
404 License: GPL-3
405
406 Files: debian/*
407-Copyright: 2013 Canonical Ltd.
408+Copyright: 2014 Canonical Ltd.
409 License: LGPL-3
410
411 License: GPL-3

Subscribers

People subscribed via source and target branches

to status/vote changes: