Ubuntu Calendar App

Merge lp:~mihirsoni/ubuntu-calendar-app/1295941 into lp:ubuntu-calendar-app

  1. 1295941
  2. Merge into trunk
Proposed by Mihir Soni on 2014-03-27
Status: Merged
Approved by: Kunal Parmar on 2014-04-04
Approved revision: 227
Merged at revision: 231
Proposed branch: lp:~mihirsoni/ubuntu-calendar-app/1295941
Merge into: lp:ubuntu-calendar-app
Diff against target: 24 lines (+5/-2)
1 file modified
EventDetails.qml (+5/-2)
To merge this branch: bzr merge lp:~mihirsoni/ubuntu-calendar-app/1295941
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2014-04-01
Kunal Parmar 2014-03-27 Needs Fixing on 2014-03-27
Nekhelesh Ramananthan Approve on 2014-03-27
Review via email: mp+212998@code.launchpad.net

Commit message

Resolved bug #1295939 & bug #1295941

Description of the change

Resolved bug #1295939 & bug #1295941

To post a comment you must log in.
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Nekhelesh Ramananthan (nik90) wrote :

Code looks good to me. However I am questioning the inclusion of the checkboxes near the guest names. Currently they all appear disabled and that gives me the impression that those guests are not attending the event. Why not remove the checkboxes and just show the guest names?

review: Needs Information
Mihir Soni (mihirsoni) wrote :

Nik,

They will be checked , unchecked based on the guest acceptance of an event.
That will be done once we implement that accepting functionality of an event.

Nekhelesh Ramananthan (nik90) wrote :

Ah okay, that makes sense. lgtm!

review: Approve
Kunal Parmar (pkunal-parmar) wrote :

+ var names = attendees[0].name.split(',');
11 + for( var j = 0 ; j < names.length ; ++j ) {
12 + contactModel.append( {"name": names[j] } );

Why we are using first attendees only ?

review: Needs Fixing
Mihir Soni (mihirsoni) wrote :

Kunal,

All the names belongs to the first entry.
If i have two guests added. it print like

Bob,Joe
Bob,Joe.

They weren't splitting actually.

Kunal Parmar (pkunal-parmar) wrote :

1 + CheckBox{
22 + enabled: false
23 + }

you can use EventAttendee's participationStatus property instead of hard coding it to false.

As of now it may not work, but no change will required from calender when it starts working

review: Needs Fixing
Kunal Parmar (pkunal-parmar) wrote :

BTw, you can refer following doc for more information
http://developer.ubuntu.com/api/qml/sdk-1.0/QtOrganizer.EventAttendee/

Kunal Parmar (pkunal-parmar) wrote :

> Kunal,
>
> All the names belongs to the first entry.
> If i have two guests added. it print like
>
> Bob,Joe
> Bob,Joe.
>
> They weren't splitting actually.

Looks like we need to handle it properly when we are adding guest entries to Event in NewEvent.

Current code in EventDetail is what it should be, no need to put hack here.

review: Needs Fixing
Mihir Soni (mihirsoni) wrote :

Are we using different separator then ??
i tried (,) and (;) none of them worked.

I guess then instead of doing this.

event.attendees = []; // if Edit remove all attendes & add them again if any
            if( personEdit.text != "") {
                var attendee = Qt.createQmlObject("import QtOrganizer 5.0; EventAttendee{}", event, "NewEvent.qml");
                attendee.name = personEdit.text;
                event.setDetail(attendee);
            }

we need to insert them separately.

Kunal Parmar (pkunal-parmar) wrote :

Current code handles only one guest.

We need to allow entry of multiple guests in NewEvent,
So when user press enter key( or we can add some button for it) on Guest entry box, we should move it to listbox and empty the guest entry box.

using this or some better approach from design team can allow multiple entry of guests.

Mihir Soni (mihirsoni) wrote :

Kunal,

I have reverted changes for splitting guests , and made modification for participation status.

I'll push separate MR for multiple guest.

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

it looks fine now, but we also need to make CheckBox read only.

Currently user is still able to select/unselect it, as bug description mention.

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

you can use enabled=false to make it readonly.

Kunal Parmar (pkunal-parmar) wrote :

ok, I see you already did that, thanks :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'EventDetails.qml'
2--- EventDetails.qml 2014-03-28 17:49:22 +0000
3+++ EventDetails.qml 2014-04-01 06:53:35 +0000
4@@ -58,7 +58,7 @@
5 contactModel.clear();
6 if( attendees !== undefined ) {
7 for( var j = 0 ; j < attendees.length ; ++j ) {
8- contactModel.append( {"name": attendees[j].name } );
9+ contactModel.append( {"name": attendees[j].name,"participationStatus": attendees[j].participationStatus } );
10 }
11 }
12
13@@ -225,7 +225,10 @@
14 }
15 delegate: Row{
16 spacing: units.gu(1)
17- CheckBox{}
18+ CheckBox{
19+ checked: participationStatus
20+ enabled: false
21+ }
22 Label {
23 text:name
24 anchors.verticalCenter: parent.verticalCenter

Subscribers

People subscribed via source and target branches

to status/vote changes: