Ubuntu File Manager App

Merge lp:~dpm/ubuntu-filemanager-app/merge-settings into lp:ubuntu-filemanager-app

  1. merge-settings
  2. Merge into trunk
Proposed by David Planella
Status: Needs review
Proposed branch: lp:~dpm/ubuntu-filemanager-app/merge-settings
Merge into: lp:ubuntu-filemanager-app
Diff against target: 604 lines (+137/-367)
5 files modified
src/app/qml/filemanager.qml (+8/-29)
src/app/qml/ui/FolderListPage.qml (+6/-15)
src/app/qml/ui/PlacesPopover.qml (+0/-149)
src/app/qml/ui/SettingsPage.qml (+123/-19)
src/app/qml/ui/ViewPopover.qml (+0/-155)
To merge this branch: bzr merge lp:~dpm/ubuntu-filemanager-app/merge-settings
Reviewer Review Type Date Requested Status
Jenkins Bot continuous-integration Needs Fixing
Arto Jalkanen Needs Fixing
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+240543@code.launchpad.net

Commit message

Merges the view options and the current settings view into a single settings page.

Description of the change

Merges the view options and the current settings view into a single settings page.

I've done some cleanup and removed some dead code. However, one related thing that still needs to be fixed is how settings are stored and the UI updated when they change. I see two issues with the current code:
- Settings are properties of either MainView or the FolderListPage component. This makes it a bit unwieldy to set from the Settings page. They should all probably be properties of one single component.
- The way to put settings into effect once they've been changed from the Settings page is to use the reloadSettings() function. Is there a more efficient way to trigger an update of the properties?

Any feedback will be really welcome!

Notes:
- Not an issue, but the use of a dark theme triggers bug 1389112 (UbuntuShape shown for ItemSelector)
- Not a big issue, but again the use of a dark theme triggers bug 1389115 (ItemSelector ticks not visible)
- Wrapping the settings into i18n.tr() is not a good idea, as if a translation is done after one particular release, then the database will be out of sync with the UI. Need to find a solution to store the settings in English while showing them translated in the UI.

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:321
http://91.189.93.70:8080/job/ubuntu-filemanager-app-ci/379/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1443
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1443/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-filemanager-app-utopic-amd64-ci/175

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-filemanager-app-ci/379/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Arto Jalkanen (ajalkane) wrote :

I tried changing (in tablet mode, using List view) for example to "Sort by date". It had no effect. I tried changing "Sort order" - no change.

Another problem is that these settings are not accessible at all in "phone" mode. Probably an artifact of the former "feature" that "Settings" page was only available in "tablet" mode.

review: Needs Fixing
Reply
lp:~dpm/ubuntu-filemanager-app/merge-settings updated
322. By David Planella

Made settings visible on phone mode

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:322
http://91.189.93.70:8080/job/ubuntu-filemanager-app-ci/380/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1448
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1448/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-filemanager-app-utopic-amd64-ci/176

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-filemanager-app-ci/380/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Arto Jalkanen (ajalkane) wrote :

Now settings available in phone mode, but there's still the problem that changing sort mode doesn't work (in neither tablet nor phone mode).

Console gets this error when trying to change those Settings:

file:///home/arto/coding/review/build-merge-settings-Desktop-Default/src/app/qml/ui/SettingsPage.qml:95: ReferenceError: fileView is not defined

Reply
Revision history for this message
Arto Jalkanen (ajalkane) :
review: Needs Fixing
Reply
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:322
https://core-apps-jenkins.ubuntu.com/job/filemanager-app-ci/16/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/52/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/filemanager-app-ci/16/rebuild

review: Needs Fixing (continuous-integration)
Reply

Unmerged revisions

322. By David Planella

Made settings visible on phone mode

321. By David Planella

Removed dead code, wrapped settings in a flickable, created layout for settings

320. By David Planella

Merged different settings into one page

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/qml/filemanager.qml'
2--- src/app/qml/filemanager.qml 2014-10-28 14:29:20 +0000
3+++ src/app/qml/filemanager.qml 2014-11-05 05:37:10 +0000
4@@ -76,16 +76,6 @@
5 serviceName: "filemanager"
6 }
7
8- // HUD Actions
9- Action {
10- id: settingsAction
11- text: i18n.tr("Settings")
12- description: i18n.tr("Change app settings")
13- iconSource: getIcon("settings")
14- onTriggered: showSettings()
15- }
16- actions: [settingsAction]
17-
18 property var pageStack: pageStack
19
20 property var folderTabs: [userplaces.locationHome]
21@@ -164,15 +154,10 @@
22 folder: userplaces.locationHome //modelData
23 }
24 }
25- Tab {
26- title: "page.title"
27- page: Page {
28- objectName: "settingsPage"
29- }
30- }
31- Tab {
32- title: "page.title"
33- page: SettingsSheet {
34+
35+ Tab {
36+ title: "page.title"
37+ page: SettingsPage {
38 id: settingsPage
39 }
40 }
41@@ -215,15 +200,17 @@
42 create: true
43
44 defaults: {
45+ showHiddenFiles: false
46 showAdvancedFeatures: false
47 collapsedSidebar: false
48+ viewMethod: i18n.tr("List")
49 }
50 }
51
52 // Individual settings, used for bindings
53 property bool showAdvancedFeatures: false
54
55- property var viewMethod
56+ property var viewMethod: getSetting("viewMethod", wideAspect ? i18n.tr("Icons") : i18n.tr("List"))
57
58 property bool collapsedSidebar: false
59
60@@ -251,12 +238,8 @@
61 }
62 }
63
64- function showSettings() {
65- PopupUtils.open(Qt.resolvedUrl("ui/SettingsSheet.qml"), mainView)
66- }
67-
68 function reloadSettings() {
69- //showAdvancedFeatures = getSetting("showAdvancedFeatures", false)
70+ showAdvancedFeatures = getSetting("showAdvancedFeatures", false)
71 viewMethod = getSetting("viewMethod", wideAspect ? i18n.tr("Icons") : i18n.tr("List"))
72 collapsedSidebar = getSetting("collapsedSidebar", false)
73 }
74@@ -265,10 +248,6 @@
75 reloadSettings()
76 }
77
78- function getIcon(name) {
79- return "/usr/share/icons/ubuntu-mobile/actions/scalable/" + name + ".svg" //Qt.resolvedUrl("icons/" + name + ".png")
80- }
81-
82 function error(title, message) {
83 PopupUtils.open(Qt.resolvedUrl("NotifyDialog.qml"), mainView,
84 {
85
86=== modified file 'src/app/qml/ui/FolderListPage.qml'
87--- src/app/qml/ui/FolderListPage.qml 2014-11-01 23:44:58 +0000
88+++ src/app/qml/ui/FolderListPage.qml 2014-11-05 05:37:10 +0000
89@@ -69,12 +69,11 @@
90 }
91 },
92 Action {
93- id: optionsButton
94- iconName: "view-list-symbolic"
95- text: i18n.tr("Properties")
96- onTriggered: {
97- PopupUtils.open(Qt.resolvedUrl("ViewPopover.qml"), parent)
98- }
99+ id: settingsButton
100+ iconName: "settings"
101+ objectName: "settings"
102+ text: i18n.tr("Settings")
103+ onTriggered: pageStack.push(settingsPage);
104 },
105 Action {
106 id: createNewFolder
107@@ -96,14 +95,6 @@
108 }
109 },
110 Action {
111- id: settingsButton
112- iconName: "settings"
113- objectName: "settings"
114- text: i18n.tr("Settings")
115- visible: sidebar.expanded
116- onTriggered: pageStack.push(settingsPage);
117- },
118- Action {
119 id: gotoButton
120 iconName: "find"
121 text: i18n.tr("Go To")
122@@ -139,7 +130,7 @@
123 flickable: !sidebar.expanded ?
124 (folderListView.visible ? folderListView : folderIconView.flickable) : null
125
126- property variant fileView: folderListPage
127+ property alias fileView: folderListPage
128 property bool showHiddenFiles: false
129 property bool showingListView: folderListView.visible
130 property string sortingMethod: "Name"
131
132=== removed file 'src/app/qml/ui/PlacesPopover.qml'
133--- src/app/qml/ui/PlacesPopover.qml 2014-10-20 21:19:26 +0000
134+++ src/app/qml/ui/PlacesPopover.qml 1970-01-01 00:00:00 +0000
135@@ -1,149 +0,0 @@
136-/*
137- * Copyright (C) 2013, 2014 Canonical Ltd
138- *
139- * This program is free software: you can redistribute it and/or modify
140- * it under the terms of the GNU General Public License version 3 as
141- * published by the Free Software Foundation.
142- *
143- * This program is distributed in the hope that it will be useful,
144- * but WITHOUT ANY WARRANTY; without even the implied warranty of
145- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
146- * GNU General Public License for more details.
147- *
148- * You should have received a copy of the GNU General Public License
149- * along with this program. If not, see <http://www.gnu.org/licenses/>.
150- *
151- * Authored by: Michael Spencer <sonrisesoftware@gmail.com>
152- */
153-import QtQuick 2.3
154-import Ubuntu.Components 1.1
155-import Ubuntu.Components.Popups 1.0
156-import Ubuntu.Components.ListItems 1.0
157-import com.ubuntu.PlacesModel 0.1
158-
159-Popover {
160- id: root
161- objectName: "placesPopover"
162-
163- Column {
164- // Places must not be visible when virtual keyboard is open, because then
165- // the virtual keyboard can push the text input out of visible area and
166- // you don't see what you type. So when virtual keyboard is open we show
167- // only the text input and a "Places" icon that you can click to get the
168- // full list again.
169- Connections {
170- id: placesVisibleController
171- target: Qt.inputMethod
172- onVisibleChanged: {
173- showPlaces.visible = Qt.inputMethod.visible
174- placesList.visible = !Qt.inputMethod.visible
175- }
176- }
177-
178- anchors {
179- left: parent.left
180- right: parent.right
181- top: parent.top
182- }
183-
184- Empty {
185-
186- TextField {
187- id: locationField
188- objectName: "inputField"
189- anchors {
190- verticalCenter: parent.verticalCenter
191- left: parent.left
192- right: goButton.left
193- margins: units.gu(1)
194- }
195-
196- inputMethodHints: Qt.ImhNoAutoUppercase
197-
198- property bool valid: pathExists(text)
199-
200- text: fileView.folder
201-
202- placeholderText: i18n.tr("Location...")
203-
204- onAccepted: goButton.clicked()
205- }
206-
207- Button {
208- id: goButton
209- objectName: "okButton"
210- anchors {
211- top: locationField.top
212- bottom: locationField.bottom
213- right: parent.right
214- rightMargin: units.gu(1)
215- }
216-
217- text: i18n.tr("Go")
218- enabled: locationField.acceptableInput && locationField.valid
219-
220- onClicked: {
221- print("User switched to:", locationField.text)
222- goTo(locationField.text)
223- PopupUtils.close(root)
224- }
225- }
226- }
227-
228- Empty {
229- id: showPlaces
230- visible: false
231- Standard {
232- objectName: "showPlaces"
233-
234- Label {
235- anchors.left: parent.left
236- anchors.leftMargin: units.gu(8)
237- anchors.verticalCenter: parent.verticalCenter
238- text: i18n.tr("Places")
239- color: Theme.palette.normal.overlayText
240- }
241-
242- onClicked: {
243- locationField.activeFocus = false
244- }
245-
246- iconSource: getIcon("location")
247-
248- iconFrame: false
249- }
250- }
251-
252- Repeater {
253- id: placesList
254- objectName: "placesList"
255- visible: true
256- model: PlacesModel {}
257-
258- delegate: Standard {
259- visible: placesList.visible
260- objectName: "place" + folderDisplayName(path).replace(/ /g,'')
261- property string name: folderDisplayName(path)
262-
263- Label {
264- anchors.left: parent.left
265- anchors.leftMargin: units.gu(8)
266- anchors.verticalCenter: parent.verticalCenter
267- text: folderDisplayName(path)
268- color: selected ? UbuntuColors.orange : Theme.palette.normal.overlayText
269- }
270-
271- iconSource: model.icon || fileIcon(model.path, true)
272-
273- onClicked: {
274- PopupUtils.close(root)
275- goTo(model.path)
276- }
277-
278- selected: folder === path
279- iconFrame: false
280- showDivider: index < (placesList.count - 1)
281- }
282- }
283- }
284-}
285
286=== renamed file 'src/app/qml/ui/SettingsSheet.qml' => 'src/app/qml/ui/SettingsPage.qml'
287--- src/app/qml/ui/SettingsSheet.qml 2014-10-18 00:35:24 +0000
288+++ src/app/qml/ui/SettingsPage.qml 2014-11-05 05:37:10 +0000
289@@ -17,8 +17,7 @@
290 */
291 import QtQuick 2.3
292 import Ubuntu.Components 1.1
293-import Ubuntu.Components.ListItems 1.0
294-import Ubuntu.Components.Popups 1.0
295+import Ubuntu.Components.ListItems 1.0 as ListItem
296
297 /*
298 * The Settings page holds global settings/preferences.
299@@ -30,23 +29,128 @@
300 id: sheet
301 title: i18n.tr("Settings")
302
303- Component.onCompleted: {
304- sheet.__leftButton.text = i18n.tr("Close")
305- sheet.__foreground.style = Theme.createStyleComponent(Qt.resolvedUrl("../components/SuruSheetStyle.qml"), sheet)
306- }
307-
308- Column {
309- anchors.fill: parent
310- anchors.margins: units.gu(-1)
311-
312- Standard {
313- text: i18n.tr("Show Advanced Features")
314- control: CheckBox {
315- id: showAdvancedFeaturesCheckBox
316- checked: showAdvancedFeatures
317- onCheckedChanged: {
318- saveSetting("showAdvancedFeatures", showAdvancedFeaturesCheckBox.checked ? "true" : "false");
319- }
320+ Flickable {
321+ id: flickable
322+ anchors {
323+ fill: parent
324+ topMargin: units.gu(1)
325+ bottomMargin: units.gu(1)
326+ }
327+ contentWidth: parent.width
328+ contentHeight: parent.height
329+ boundsBehavior: (contentHeight > parent.height) ?
330+ Flickable.DragAndOvershootBounds : Flickable.StopAtBounds
331+
332+ Column {
333+ id: content
334+ spacing: units.gu(1)
335+ anchors {
336+ margins: units.gu(3)
337+ left: parent.left
338+ right: parent.right
339+ }
340+
341+ ListItem.Standard {
342+ text: i18n.tr("Show Hidden Files")
343+ control: CheckBox {
344+ id: showHiddenFilesCheckBox
345+ objectName: "showHiddenFileCheckBox"
346+ checked: getSetting("showHiddenFiles", false);
347+ onCheckedChanged: {
348+ saveSetting("showHiddenFiles",
349+ showHiddenFilesCheckBox.checked ? true : false);
350+ fileView.showHiddenFiles = checked
351+ }
352+ }
353+ }
354+
355+ ListItem.ItemSelector {
356+ text: i18n.tr("View As")
357+
358+ expanded: true
359+ delegate: selectorDelegate
360+ selectedIndex: model.indexOf(mainView.viewMethod)
361+ model: [
362+ i18n.tr("Icons"),
363+ i18n.tr("List")
364+ ]
365+
366+ onSelectedIndexChanged: {
367+ mainView.viewMethod = model[selectedIndex]
368+ saveSetting("viewMethod", model[selectedIndex])
369+ }
370+ }
371+
372+ ListItem.ItemSelector {
373+ text: i18n.tr("Sort By")
374+ expanded: true
375+ width: parent.width
376+ selectedIndex: values.indexOf(fileView.sortingMethod)
377+ model: [
378+ i18n.tr("Name"),
379+ i18n.tr("Date")
380+ ]
381+
382+ onSelectedIndexChanged: {
383+ fileView.sortingMethod = values[selectedIndex]
384+ }
385+ }
386+
387+ ListItem.ItemSelector {
388+ text: i18n.tr("Sort Order")
389+ expanded: true
390+ selectedIndex: sortAscending ? 0 : 1
391+ model: [
392+ i18n.tr("Ascending"),
393+ i18n.tr("Descending")
394+ ]
395+
396+ onSelectedIndexChanged: {
397+ fileView.sortAscending = (values[selectedIndex] === i18n.tr("Ascending"))
398+ }
399+ }
400+
401+ ListItem.Standard {
402+ text: i18n.tr("Show Advanced Features")
403+ control: CheckBox {
404+ id: showAdvancedFeaturesCheckBox
405+ checked: getSetting("showAdvancedFeatures", false);
406+ onCheckedChanged: {
407+ saveSetting("showAdvancedFeatures",
408+ showAdvancedFeaturesCheckBox.checked ? true : false);
409+ }
410+ }
411+ }
412+
413+ ListItem.Standard {
414+ text: i18n.tr("Filter")
415+
416+ visible: fileView.showAdvancedFeatures
417+
418+ control: TextField {
419+ id: filterField
420+ anchors {
421+ verticalCenter: parent.verticalCenter
422+ right: parent.right
423+ margins: units.gu(1)
424+ }
425+
426+ inputMethodHints: Qt.ImhNoAutoUppercase
427+
428+ text: pageModel.nameFilters
429+
430+ onAccepted: goButton.clicked()
431+ onTextChanged: {
432+ if (text !== pageModel.nameFilters)
433+ pageModel.nameFilters = [text]
434+ }
435+ }
436+ }
437+
438+ Component {
439+ id: selectorDelegate
440+
441+ OptionSelectorDelegate { showDivider: false; }
442 }
443 }
444 }
445
446=== removed file 'src/app/qml/ui/ViewPopover.qml'
447--- src/app/qml/ui/ViewPopover.qml 2014-09-20 10:49:51 +0000
448+++ src/app/qml/ui/ViewPopover.qml 1970-01-01 00:00:00 +0000
449@@ -1,155 +0,0 @@
450-/*
451- * Copyright (C) 2013 Canonical Ltd
452- *
453- * This program is free software: you can redistribute it and/or modify
454- * it under the terms of the GNU General Public License version 3 as
455- * published by the Free Software Foundation.
456- *
457- * This program is distributed in the hope that it will be useful,
458- * but WITHOUT ANY WARRANTY; without even the implied warranty of
459- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
460- * GNU General Public License for more details.
461- *
462- * You should have received a copy of the GNU General Public License
463- * along with this program. If not, see <http://www.gnu.org/licenses/>.
464- *
465- * Authored by: Michael Spencer <sonrisesoftware@gmail.com>
466- */
467-import QtQuick 2.3
468-import Ubuntu.Components 1.1
469-import Ubuntu.Components.Popups 1.0
470-import Ubuntu.Components.ListItems 1.0
471-
472-Popover {
473- id: root
474- objectName: "viewPopover"
475-
476- Column {
477- anchors {
478- left: parent.left
479- right: parent.right
480- top: parent.top
481- }
482-
483- Standard {
484- id: showHiddenFileCheckBox
485- objectName: "showHiddenFileCheckBox"
486-
487- Label {
488- text: i18n.tr("Show Hidden Files")
489- fontSize: "medium"
490- color: Theme.palette.normal.overlayText
491- anchors.left: parent.left
492- anchors.leftMargin: units.gu(2)
493- anchors.verticalCenter: parent.verticalCenter
494- }
495-
496- control: CheckBox {
497- anchors.verticalCenter: parent.verticalCenter
498-
499- checked: fileView.showHiddenFiles
500- onCheckedChanged: {
501- fileView.showHiddenFiles = checked
502- }
503- }
504- }
505-
506- ValueSelector {
507- Label {
508- text: i18n.tr("View As")
509- fontSize: "medium"
510- color: Theme.palette.normal.overlayText
511- anchors.left: parent.left
512- anchors.leftMargin: units.gu(2)
513- anchors.top: parent.top
514- anchors.topMargin: units.gu(1.6)
515- }
516-
517- selectedIndex: values.indexOf(viewMethod)
518- values: [
519- i18n.tr("Icons"),
520- i18n.tr("List")
521- ]
522-
523- onSelectedIndexChanged: {
524- saveSetting("viewMethod", values[selectedIndex])
525- }
526- }
527-
528- ValueSelector {
529- Label {
530- text: i18n.tr("Sort By")
531- fontSize: "medium"
532- color: Theme.palette.normal.overlayText
533- anchors.left: parent.left
534- anchors.leftMargin: units.gu(2)
535- anchors.top: parent.top
536- anchors.topMargin: units.gu(1.6)
537- }
538-
539- selectedIndex: values.indexOf(fileView.sortingMethod)
540- values: [
541- i18n.tr("Name"),
542- i18n.tr("Date")
543- ]
544-
545- onSelectedIndexChanged: {
546- fileView.sortingMethod = values[selectedIndex]
547- }
548- }
549-
550- ValueSelector {
551- Label {
552- text: i18n.tr("Sort Order")
553- fontSize: "medium"
554- color: Theme.palette.normal.overlayText
555- anchors.left: parent.left
556- anchors.leftMargin: units.gu(2)
557- anchors.top: parent.top
558- anchors.topMargin: units.gu(1.7)
559- }
560-
561- selectedIndex: sortAscending ? 0 : 1
562- values: [
563- i18n.tr("Ascending"),
564- i18n.tr("Descending")
565- ]
566-
567- onSelectedIndexChanged: {
568- fileView.sortAscending = (values[selectedIndex] === i18n.tr("Ascending"))
569- }
570- }
571-
572- Standard {
573- visible: showAdvancedFeatures
574-
575- Label {
576- text: i18n.tr("Filter")
577- fontSize: "medium"
578- color: Theme.palette.normal.overlayText
579- anchors.left: parent.left
580- anchors.leftMargin: units.gu(2)
581- anchors.verticalCenter: parent.verticalCenter
582- }
583-
584- TextField {
585- id: filterField
586- anchors {
587- verticalCenter: parent.verticalCenter
588- right: parent.right
589- margins: units.gu(1)
590- }
591-
592- inputMethodHints: Qt.ImhNoAutoUppercase
593-
594- text: pageModel.nameFilters
595-
596- onAccepted: goButton.clicked()
597- onTextChanged: {
598- if (text !== pageModel.nameFilters)
599- pageModel.nameFilters = [text]
600- }
601- }
602- }
603- }
604-}

Subscribers

People subscribed via source and target branches