Merge lp:~qqworini/ubuntu-rssreader-app/fix-toolbar-bug into lp:~ubuntu-shorts-dev/ubuntu-rssreader-app/trunk

Proposed by Joey Chan
Status: Merged
Approved by: Roman Shchekin
Approved revision: 72
Merged at revision: 73
Proposed branch: lp:~qqworini/ubuntu-rssreader-app/fix-toolbar-bug
Merge into: lp:~ubuntu-shorts-dev/ubuntu-rssreader-app/trunk
Diff against target: 45 lines (+8/-2)
2 files modified
feeds/AppendFeedPage.qml (+4/-1)
feeds/ChooseTopicPage.qml (+4/-1)
To merge this branch: bzr merge lp:~qqworini/ubuntu-rssreader-app/fix-toolbar-bug
Reviewer Review Type Date Requested Status
David Planella Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+187976@code.launchpad.net

Commit message

fix bug #1231137: Missing "Next" button while adding feed (desktop only)

Description of the change

fix bug #1231137: Missing "Next" button while adding feed (desktop only)

// code like below will produce this bug
Page{
tool: ToolbarItems { // ToolbarItems initial with the page
        id: toolbar

        opened: true // bug from here, never open in desktop
        locked: true // if set lock to false, toolbar can be swiped up

// code like below solve temporarily
Page{
id: page
tool: null // set the tool to null at initialization

ToolbarItems {
        id: toolbar

        opened: true
        locked: true
}

function setTool() {
       page.tools = toolbar // set toolbar manually can solve
}

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)
Revision history for this message
David Planella (dpm) wrote :

Nice work Joey. It looks good to me as a workaround until the bug in the SDK is fixed, and Jenkins is happy with it.

Just one thing: could you add a comment in the code, referencing this bug? This way in a few weeks time, when none of us remembers why we did this, it'll help us recall the motivation :)

E.g.

// Workaround for bug #1231137

9 + tools: null

review: Approve
Revision history for this message
Roman Shchekin (mrqtros) wrote :

It works!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'feeds/AppendFeedPage.qml'
2--- feeds/AppendFeedPage.qml 2013-09-21 09:27:59 +0000
3+++ feeds/AppendFeedPage.qml 2013-09-27 05:18:31 +0000
4@@ -20,7 +20,9 @@
5 //.property var selectedFeeds: null // Become local.
6 property bool resultsReceived: false // Indicates that at least once results were received.
7
8- tools: ToolbarItems {
9+ tools: null
10+
11+ ToolbarItems {
12 id: appendFeedTools
13
14 opened: true
15@@ -78,6 +80,7 @@
16 }
17
18 function reloadPageContent() {
19+ appendFeedPage.tools = appendFeedTools
20 if (isDirty) {
21 tfFeedUrl.text = ""
22 resultsReceived = false
23
24=== modified file 'feeds/ChooseTopicPage.qml'
25--- feeds/ChooseTopicPage.qml 2013-09-11 15:18:51 +0000
26+++ feeds/ChooseTopicPage.qml 2013-09-27 05:18:31 +0000
27@@ -17,7 +17,9 @@
28
29 signal topicChoosen(int topicId, var addedFeeds)
30
31- tools: ToolbarItems {
32+ tools: null
33+
34+ ToolbarItems {
35 id: appendFeedTools
36
37 opened: true
38@@ -37,6 +39,7 @@
39 }
40
41 function reloadPageContent() {
42+ chooseTopicPage.tools = appendFeedTools
43 suggestionTopicsModel.clear()
44
45 var tags = DB.loadTags()

Subscribers

People subscribed via source and target branches