Merge lp:~renatofilho/quick-memo/fix-item-focus into lp:quick-memo/trunk

Proposed by Renato Araujo Oliveira Filho
Status: Merged
Approved by: Stefano Verzegnassi
Approved revision: 57
Merged at revision: 57
Proposed branch: lp:~renatofilho/quick-memo/fix-item-focus
Merge into: lp:quick-memo/trunk
Diff against target: 189 lines (+87/-30)
2 files modified
app/components/ListManager.qml (+77/-18)
app/components/NoteTextField.qml (+10/-12)
To merge this branch: bzr merge lp:~renatofilho/quick-memo/fix-item-focus
Reviewer Review Type Date Requested Status
Stefano Verzegnassi Approve
Bill Filler (community) Approve
Review via email: mp+245761@code.launchpad.net

Commit message

Fixed note items navigation and focus.

Description of the change

OBS: The keyboard "Enter" label still broken due a bug on ubuntu-keyboard, you will need this MR to get this fully working: https://code.launchpad.net/~renatofilho/ubuntu-keyboard/fix-qml-inputMethod-extensions/+merge/245775

To post a comment you must log in.
57. By Renato Araujo Oliveira Filho

Fixed note itens navigation and focus.

Revision history for this message
Stefano Verzegnassi (verzegnassi-stefano) wrote :

Hi Renato,

Thanks for reporting this issue and submitting a patch.
During these days I'm studying for some exams, so I don't have much time to spend with source code.

I'm working on the "reboot" branch for Quick Memo, using a new database (in order to solve some issue with U1DB) and refactoring some parts of code that don't look so good ("Details" page is one of those "parts").

I just took a look at the diff and the code looks good to me. I just need some time to make a deeper review of the code and integrate it in the "reboot" branch (which, I hope, should be "almost" ready to be receive MPs during the next weekend).

Thank you again! :)
Stefano

Revision history for this message
Bill Filler (bfiller) wrote :

I tested this and it works well with ubuntu-keyboard in silo 19

review: Approve
Revision history for this message
Bill Filler (bfiller) wrote :

Stefano,
Wondering if you had plans to release a new version before the reboot branch? If so would be great to get this merged and released prior to the reboot branch as these changes really help usability alot. I use the app all the time (it's awesome!) and these make it even better.

Revision history for this message
Stefano Verzegnassi (verzegnassi-stefano) wrote :

Bill,
Sorry for the late replay, I didn't get any notification by email about your comment.
I had no plan about an "intermediate" release, but I can definitely do it.
Just need some time to fix some issue with in-app notifications (hopefully it should be done during this week).

Renato,
Thank you very much for this contribution! It's really a great improvement! :D

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/components/ListManager.qml'
2--- app/components/ListManager.qml 2014-11-03 00:21:23 +0000
3+++ app/components/ListManager.qml 2015-01-07 18:28:07 +0000
4@@ -29,8 +29,8 @@
5 property Flickable flickable
6 property alias model: repeater.model
7 property alias delegate: repeater.delegate
8-
9 property alias footer: footerLoader.sourceComponent
10+ readonly property bool focusOnLastItem: (model.focusedIndex === -1) || (model.focusedIndex === (model.count - 1))
11
12 signal listChanged()
13
14@@ -44,6 +44,8 @@
15
16 model: ListModel {
17 id: dataModel
18+
19+ property int focusedIndex: -1
20 }
21
22 delegate: delegate
23@@ -99,7 +101,7 @@
24 }
25
26 onClicked: {
27- rootItem.model.append({"checked": false, "text": ""})
28+ rootItem.addItem({"checked": false, "text": ""})
29 // Here we don't send listChanged signal, since it is an empty item that should not be saved
30 }
31 }
32@@ -120,6 +122,64 @@
33 return list
34 }
35
36+ function addItem(args) {
37+ rootItem.model.append(args)
38+ var newItem = repeater.itemAt(repeater.count - 1)
39+ newItem.textArea.forceActiveFocus()
40+ autoScrollAnimation.makeMeVisible(newItem)
41+ }
42+
43+ SequentialAnimation {
44+ id: autoScrollAnimation
45+
46+ property var targetItem: null
47+ alwaysRunToEnd: true
48+
49+ // wait item be moved to correct place
50+ PauseAnimation {
51+ duration: 100
52+ }
53+ // scroll to new item position
54+ ScriptAction {
55+ script: {
56+ if (autoScrollAnimation.targetItem) {
57+ autoScrollAnimation.makeMeVisibleImpl(autoScrollAnimation.targetItem)
58+ autoScrollAnimation.targetItem = null
59+ }
60+ }
61+ }
62+
63+ function makeMeVisible(newItem) {
64+ autoScrollAnimation.targetItem = newItem
65+ autoScrollAnimation.restart()
66+ }
67+
68+ function makeMeVisibleImpl(newItem) {
69+ if (!newItem) {
70+ return
71+ }
72+
73+ var positionY = rootItem.y + repeater.y + newItem.y
74+
75+ // check if the item is already visible
76+ var bottomY = flickable.contentY + flickable.height
77+ var itemBottom = positionY + (newItem.height *3) // margin
78+ if (positionY >= flickable.contentY && itemBottom <= bottomY) {
79+ return;
80+ }
81+
82+ // if it is not, try to scroll and make it visible
83+ var targetY = itemBottom - flickable.height
84+ if (targetY >= 0 && positionY) {
85+ flickable.contentY = targetY
86+ } else if (positionY < flickable.contentY) {
87+ // if it is hidden at the top, also show it
88+ flickable.contentY = positionY
89+ }
90+ flickable.returnToBounds()
91+ }
92+ }
93+
94
95 // *** DELEGATE ***
96 Component {
97@@ -197,17 +257,14 @@
98
99 NoteTextField {
100 id: textArea
101+
102 width: parent.width - (checkBox.width + (parent.spacing * 2))
103 anchors.verticalCenter: parent.verticalCenter
104
105 font.strikeout: checkBox.checked
106
107- __inverseMouseArea.parent: item
108- onFocusLost: {
109- if (rootItem.flickable)
110- rootItem.flickable.forceActiveFocus()
111- else
112- rootItem.forceActiveFocus()
113+ onFocusReceived: {
114+ rootItem.model.focusedIndex = model.index
115 }
116
117 text: model.text
118@@ -222,23 +279,25 @@
119 }
120
121 //hasClearButton: false
122- focus: false
123+ focus: true
124
125 // Ubuntu Keyboard
126+ // TODO: Disable Enter key if model.text is empty.
127 InputMethod.extensions: {
128- "enterKeyText": (model.index < (rootItem.model.count - 1)) ? i18n.tr("Next") : i18n.tr("Add")
129- // TODO: Disable Enter key if model.text is empty.
130+ "enterKeyText": rootItem.focusOnLastItem ? i18n.tr("Add") : i18n.tr("Next")
131 }
132+
133 Keys.onReturnPressed: {
134- if ((model.index === (rootItem.model.count - 1)) && model.text !== "") {
135+ if (rootItem.focusOnLastItem && (model.text !== "")) {
136 // Create a new item.
137- rootItem.model.append({"checked": false, "text": ""})
138+ rootItem.addItem({"checked": false, "text": ""})
139+ } else if (!rootItem.focusOnLastItem) {
140+ var nextItem = repeater.itemAt(model.index + 1)
141+ if (nextItem) {
142+ nextItem.textArea.forceActiveFocus()
143+ autoScrollAnimation.makeMeVisible(nextItem)
144+ }
145 }
146- repeater.itemAt(model.index + 1).textArea.forceActiveFocus()
147-
148- // Focused textArea should be always visible. Move the Flickable as needed.
149- flickable.contentY = rootItem.y + repeater.itemAt(model.index + 1).y
150- flickable.returnToBounds()
151 }
152 }
153
154
155=== modified file 'app/components/NoteTextField.qml'
156--- app/components/NoteTextField.qml 2014-10-04 01:24:43 +0000
157+++ app/components/NoteTextField.qml 2015-01-07 18:28:07 +0000
158@@ -35,21 +35,19 @@
159 // Prevent data loss
160 inputMethodHints: Qt.ImhNoPredictiveText
161
162- property alias __inverseMouseArea: inverseMouseArea
163- signal focusLost()
164+ signal focusLost()
165+ signal focusReceived()
166+
167+ onActiveFocusChanged: {
168+ if (activeFocus) {
169+ focusReceived()
170+ } else {
171+ focusLost()
172+ }
173+ }
174
175 style: TextFieldStyle {
176 frameSpacing: 0
177 background: Item { anchors.fill: parent }
178 }
179-
180- InverseMouseArea {
181- id: inverseMouseArea
182- enabled: textArea.activeFocus
183- anchors.fill: parent
184- onClicked: {
185- textArea.focusLost()
186- mouse.accepted = false
187- }
188- }
189 }

Subscribers

People subscribed via source and target branches