Merge lp:~mihirsoni/ubuntu-calendar-app/ReminderDesignFix-3 into lp:ubuntu-calendar-app

Proposed by Mihir Soni
Status: Merged
Approved by: Kunal Parmar
Approved revision: 531
Merged at revision: 552
Proposed branch: lp:~mihirsoni/ubuntu-calendar-app/ReminderDesignFix-3
Merge into: lp:ubuntu-calendar-app
Diff against target: 87 lines (+25/-42)
1 file modified
EventReminder.qml (+25/-42)
To merge this branch: bzr merge lp:~mihirsoni/ubuntu-calendar-app/ReminderDesignFix-3
Reviewer Review Type Date Requested Status
Kunal Parmar Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+239598@code.launchpad.net

Commit message

Reminder Design Fix

Description of the change

Reminder Design Fix

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
528. By Mihir Soni

Fixed height issue

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

merge with trunk

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 :

Code works fine and looks good, I have few improvements comments.

Added one inline comment regarding removing column.

Also can you add ScrollBar, depending on screen size we might need it ?

review: Needs Fixing
530. By Mihir Soni

Removed column , also added scrollbar

531. By Mihir Soni

merge from trunk

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
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mihir Soni (mihirsoni) wrote :

Hi Kunal,

Could you review this MP?

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

looks good to me

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'EventReminder.qml'
--- EventReminder.qml 2014-10-10 17:49:49 +0000
+++ EventReminder.qml 2014-11-11 12:50:57 +0000
@@ -49,58 +49,41 @@
49 pop();49 pop();
50 }50 }
51 }51 }
5252 Scrollbar{
53 id:scrollList
54 flickableItem: _pageFlickable
55 anchors.fill :parent
56 }
53 Flickable {57 Flickable {
54 id: _pageFlickable58 id: _pageFlickable
5559
60
56 clip: true61 clip: true
57 anchors.fill: parent62 anchors.fill: parent
58 contentHeight: reminderModel.count * units.gu(7)63 contentHeight: _reminders.itemHeight * reminderModel.count + units.gu(2)
5964 ListItem.ItemSelector {
60 Column {65 id: _reminders
61 id: _reminderColumn66 expanded: true
6267 model: reminderModel
63 anchors {68 delegate: selectorDelegate
64 top: parent.top69 selectedIndex: reminderModel.get
65 left: parent.left70 onSelectedIndexChanged: {
66 right: parent.right71 root.reminderTime = reminderModel.get(selectedIndex).value
67 }72 }
6873
69 Repeater {74 Component.onCompleted: {
70 id: _reminders75 for(var i=0; i<reminderModel.count; i++) {
7176 if (root.reminderTime === reminderModel.get(i).value){
72 model: reminderModel77 _reminders.selectedIndex = i
7378 return;
74 ListItem.Standard {
75 id: _reminderDelegate
76
77 property alias isChecked: reminderCheckbox.checked
78
79 text: label
80 control: CheckBox {
81 id: reminderCheckbox
82
83 checked: root.reminderTime === value
84
85 onClicked: {
86 root.reminderTime = value
87 if (checked) {
88 // Ensures only one reminder option is selected
89 for(var i=0; i<reminderModel.count; i++) {
90 if(_reminders.itemAt(i).isChecked &&
91 i !== index) {
92 _reminders.itemAt(i).isChecked = false
93 }
94 }
95 }
96
97 else {
98 checked = !checked
99 }
100 }
101 }79 }
102 }80 }
103 }81 }
104 }82 }
83 Component {
84 id: selectorDelegate
85 OptionSelectorDelegate { text: label; }
86 }
87
105 }88 }
106}89}

Subscribers

People subscribed via source and target branches

to status/vote changes: