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

Proposed by David Planella on 2014-11-04
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 on 2015-10-22
Arto Jalkanen 2014-11-04 Needs Fixing on 2014-12-09
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2014-11-05
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.
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
322. By David Planella on 2014-11-05

Made settings visible on phone mode

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

Arto Jalkanen (ajalkane) :
review: Needs Fixing
review: Needs Fixing (continuous-integration)

Unmerged revisions

322. By David Planella on 2014-11-05

Made settings visible on phone mode

321. By David Planella on 2014-11-03

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

320. By David Planella on 2014-10-30

Merged different settings into one page

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/app/qml/filemanager.qml'
--- src/app/qml/filemanager.qml 2014-10-28 14:29:20 +0000
+++ src/app/qml/filemanager.qml 2014-11-05 05:37:10 +0000
@@ -76,16 +76,6 @@
76 serviceName: "filemanager"76 serviceName: "filemanager"
77 }77 }
7878
79 // HUD Actions
80 Action {
81 id: settingsAction
82 text: i18n.tr("Settings")
83 description: i18n.tr("Change app settings")
84 iconSource: getIcon("settings")
85 onTriggered: showSettings()
86 }
87 actions: [settingsAction]
88
89 property var pageStack: pageStack79 property var pageStack: pageStack
9080
91 property var folderTabs: [userplaces.locationHome]81 property var folderTabs: [userplaces.locationHome]
@@ -164,15 +154,10 @@
164 folder: userplaces.locationHome //modelData154 folder: userplaces.locationHome //modelData
165 }155 }
166 }156 }
167 Tab {157
168 title: "page.title"158 Tab {
169 page: Page {159 title: "page.title"
170 objectName: "settingsPage"160 page: SettingsPage {
171 }
172 }
173 Tab {
174 title: "page.title"
175 page: SettingsSheet {
176 id: settingsPage161 id: settingsPage
177 }162 }
178 }163 }
@@ -215,15 +200,17 @@
215 create: true200 create: true
216201
217 defaults: {202 defaults: {
203 showHiddenFiles: false
218 showAdvancedFeatures: false204 showAdvancedFeatures: false
219 collapsedSidebar: false205 collapsedSidebar: false
206 viewMethod: i18n.tr("List")
220 }207 }
221 }208 }
222209
223 // Individual settings, used for bindings210 // Individual settings, used for bindings
224 property bool showAdvancedFeatures: false211 property bool showAdvancedFeatures: false
225212
226 property var viewMethod213 property var viewMethod: getSetting("viewMethod", wideAspect ? i18n.tr("Icons") : i18n.tr("List"))
227214
228 property bool collapsedSidebar: false215 property bool collapsedSidebar: false
229216
@@ -251,12 +238,8 @@
251 }238 }
252 }239 }
253240
254 function showSettings() {
255 PopupUtils.open(Qt.resolvedUrl("ui/SettingsSheet.qml"), mainView)
256 }
257
258 function reloadSettings() {241 function reloadSettings() {
259 //showAdvancedFeatures = getSetting("showAdvancedFeatures", false)242 showAdvancedFeatures = getSetting("showAdvancedFeatures", false)
260 viewMethod = getSetting("viewMethod", wideAspect ? i18n.tr("Icons") : i18n.tr("List"))243 viewMethod = getSetting("viewMethod", wideAspect ? i18n.tr("Icons") : i18n.tr("List"))
261 collapsedSidebar = getSetting("collapsedSidebar", false)244 collapsedSidebar = getSetting("collapsedSidebar", false)
262 }245 }
@@ -265,10 +248,6 @@
265 reloadSettings()248 reloadSettings()
266 }249 }
267250
268 function getIcon(name) {
269 return "/usr/share/icons/ubuntu-mobile/actions/scalable/" + name + ".svg" //Qt.resolvedUrl("icons/" + name + ".png")
270 }
271
272 function error(title, message) {251 function error(title, message) {
273 PopupUtils.open(Qt.resolvedUrl("NotifyDialog.qml"), mainView,252 PopupUtils.open(Qt.resolvedUrl("NotifyDialog.qml"), mainView,
274 {253 {
275254
=== modified file 'src/app/qml/ui/FolderListPage.qml'
--- src/app/qml/ui/FolderListPage.qml 2014-11-01 23:44:58 +0000
+++ src/app/qml/ui/FolderListPage.qml 2014-11-05 05:37:10 +0000
@@ -69,12 +69,11 @@
69 }69 }
70 },70 },
71 Action {71 Action {
72 id: optionsButton72 id: settingsButton
73 iconName: "view-list-symbolic"73 iconName: "settings"
74 text: i18n.tr("Properties")74 objectName: "settings"
75 onTriggered: {75 text: i18n.tr("Settings")
76 PopupUtils.open(Qt.resolvedUrl("ViewPopover.qml"), parent)76 onTriggered: pageStack.push(settingsPage);
77 }
78 },77 },
79 Action {78 Action {
80 id: createNewFolder79 id: createNewFolder
@@ -96,14 +95,6 @@
96 }95 }
97 },96 },
98 Action {97 Action {
99 id: settingsButton
100 iconName: "settings"
101 objectName: "settings"
102 text: i18n.tr("Settings")
103 visible: sidebar.expanded
104 onTriggered: pageStack.push(settingsPage);
105 },
106 Action {
107 id: gotoButton98 id: gotoButton
108 iconName: "find"99 iconName: "find"
109 text: i18n.tr("Go To")100 text: i18n.tr("Go To")
@@ -139,7 +130,7 @@
139 flickable: !sidebar.expanded ?130 flickable: !sidebar.expanded ?
140 (folderListView.visible ? folderListView : folderIconView.flickable) : null131 (folderListView.visible ? folderListView : folderIconView.flickable) : null
141132
142 property variant fileView: folderListPage133 property alias fileView: folderListPage
143 property bool showHiddenFiles: false134 property bool showHiddenFiles: false
144 property bool showingListView: folderListView.visible135 property bool showingListView: folderListView.visible
145 property string sortingMethod: "Name"136 property string sortingMethod: "Name"
146137
=== removed file 'src/app/qml/ui/PlacesPopover.qml'
--- src/app/qml/ui/PlacesPopover.qml 2014-10-20 21:19:26 +0000
+++ src/app/qml/ui/PlacesPopover.qml 1970-01-01 00:00:00 +0000
@@ -1,149 +0,0 @@
1/*
2 * Copyright (C) 2013, 2014 Canonical Ltd
3 *
4 * This program is free software: you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License version 3 as
6 * published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Michael Spencer <sonrisesoftware@gmail.com>
17 */
18import QtQuick 2.3
19import Ubuntu.Components 1.1
20import Ubuntu.Components.Popups 1.0
21import Ubuntu.Components.ListItems 1.0
22import com.ubuntu.PlacesModel 0.1
23
24Popover {
25 id: root
26 objectName: "placesPopover"
27
28 Column {
29 // Places must not be visible when virtual keyboard is open, because then
30 // the virtual keyboard can push the text input out of visible area and
31 // you don't see what you type. So when virtual keyboard is open we show
32 // only the text input and a "Places" icon that you can click to get the
33 // full list again.
34 Connections {
35 id: placesVisibleController
36 target: Qt.inputMethod
37 onVisibleChanged: {
38 showPlaces.visible = Qt.inputMethod.visible
39 placesList.visible = !Qt.inputMethod.visible
40 }
41 }
42
43 anchors {
44 left: parent.left
45 right: parent.right
46 top: parent.top
47 }
48
49 Empty {
50
51 TextField {
52 id: locationField
53 objectName: "inputField"
54 anchors {
55 verticalCenter: parent.verticalCenter
56 left: parent.left
57 right: goButton.left
58 margins: units.gu(1)
59 }
60
61 inputMethodHints: Qt.ImhNoAutoUppercase
62
63 property bool valid: pathExists(text)
64
65 text: fileView.folder
66
67 placeholderText: i18n.tr("Location...")
68
69 onAccepted: goButton.clicked()
70 }
71
72 Button {
73 id: goButton
74 objectName: "okButton"
75 anchors {
76 top: locationField.top
77 bottom: locationField.bottom
78 right: parent.right
79 rightMargin: units.gu(1)
80 }
81
82 text: i18n.tr("Go")
83 enabled: locationField.acceptableInput && locationField.valid
84
85 onClicked: {
86 print("User switched to:", locationField.text)
87 goTo(locationField.text)
88 PopupUtils.close(root)
89 }
90 }
91 }
92
93 Empty {
94 id: showPlaces
95 visible: false
96 Standard {
97 objectName: "showPlaces"
98
99 Label {
100 anchors.left: parent.left
101 anchors.leftMargin: units.gu(8)
102 anchors.verticalCenter: parent.verticalCenter
103 text: i18n.tr("Places")
104 color: Theme.palette.normal.overlayText
105 }
106
107 onClicked: {
108 locationField.activeFocus = false
109 }
110
111 iconSource: getIcon("location")
112
113 iconFrame: false
114 }
115 }
116
117 Repeater {
118 id: placesList
119 objectName: "placesList"
120 visible: true
121 model: PlacesModel {}
122
123 delegate: Standard {
124 visible: placesList.visible
125 objectName: "place" + folderDisplayName(path).replace(/ /g,'')
126 property string name: folderDisplayName(path)
127
128 Label {
129 anchors.left: parent.left
130 anchors.leftMargin: units.gu(8)
131 anchors.verticalCenter: parent.verticalCenter
132 text: folderDisplayName(path)
133 color: selected ? UbuntuColors.orange : Theme.palette.normal.overlayText
134 }
135
136 iconSource: model.icon || fileIcon(model.path, true)
137
138 onClicked: {
139 PopupUtils.close(root)
140 goTo(model.path)
141 }
142
143 selected: folder === path
144 iconFrame: false
145 showDivider: index < (placesList.count - 1)
146 }
147 }
148 }
149}
1500
=== renamed file 'src/app/qml/ui/SettingsSheet.qml' => 'src/app/qml/ui/SettingsPage.qml'
--- src/app/qml/ui/SettingsSheet.qml 2014-10-18 00:35:24 +0000
+++ src/app/qml/ui/SettingsPage.qml 2014-11-05 05:37:10 +0000
@@ -17,8 +17,7 @@
17 */17 */
18import QtQuick 2.318import QtQuick 2.3
19import Ubuntu.Components 1.119import Ubuntu.Components 1.1
20import Ubuntu.Components.ListItems 1.020import Ubuntu.Components.ListItems 1.0 as ListItem
21import Ubuntu.Components.Popups 1.0
2221
23/*22/*
24 * The Settings page holds global settings/preferences.23 * The Settings page holds global settings/preferences.
@@ -30,23 +29,128 @@
30 id: sheet29 id: sheet
31 title: i18n.tr("Settings")30 title: i18n.tr("Settings")
3231
33 Component.onCompleted: {32 Flickable {
34 sheet.__leftButton.text = i18n.tr("Close")33 id: flickable
35 sheet.__foreground.style = Theme.createStyleComponent(Qt.resolvedUrl("../components/SuruSheetStyle.qml"), sheet)34 anchors {
36 }35 fill: parent
3736 topMargin: units.gu(1)
38 Column {37 bottomMargin: units.gu(1)
39 anchors.fill: parent38 }
40 anchors.margins: units.gu(-1)39 contentWidth: parent.width
4140 contentHeight: parent.height
42 Standard {41 boundsBehavior: (contentHeight > parent.height) ?
43 text: i18n.tr("Show Advanced Features")42 Flickable.DragAndOvershootBounds : Flickable.StopAtBounds
44 control: CheckBox {43
45 id: showAdvancedFeaturesCheckBox44 Column {
46 checked: showAdvancedFeatures45 id: content
47 onCheckedChanged: {46 spacing: units.gu(1)
48 saveSetting("showAdvancedFeatures", showAdvancedFeaturesCheckBox.checked ? "true" : "false");47 anchors {
49 }48 margins: units.gu(3)
49 left: parent.left
50 right: parent.right
51 }
52
53 ListItem.Standard {
54 text: i18n.tr("Show Hidden Files")
55 control: CheckBox {
56 id: showHiddenFilesCheckBox
57 objectName: "showHiddenFileCheckBox"
58 checked: getSetting("showHiddenFiles", false);
59 onCheckedChanged: {
60 saveSetting("showHiddenFiles",
61 showHiddenFilesCheckBox.checked ? true : false);
62 fileView.showHiddenFiles = checked
63 }
64 }
65 }
66
67 ListItem.ItemSelector {
68 text: i18n.tr("View As")
69
70 expanded: true
71 delegate: selectorDelegate
72 selectedIndex: model.indexOf(mainView.viewMethod)
73 model: [
74 i18n.tr("Icons"),
75 i18n.tr("List")
76 ]
77
78 onSelectedIndexChanged: {
79 mainView.viewMethod = model[selectedIndex]
80 saveSetting("viewMethod", model[selectedIndex])
81 }
82 }
83
84 ListItem.ItemSelector {
85 text: i18n.tr("Sort By")
86 expanded: true
87 width: parent.width
88 selectedIndex: values.indexOf(fileView.sortingMethod)
89 model: [
90 i18n.tr("Name"),
91 i18n.tr("Date")
92 ]
93
94 onSelectedIndexChanged: {
95 fileView.sortingMethod = values[selectedIndex]
96 }
97 }
98
99 ListItem.ItemSelector {
100 text: i18n.tr("Sort Order")
101 expanded: true
102 selectedIndex: sortAscending ? 0 : 1
103 model: [
104 i18n.tr("Ascending"),
105 i18n.tr("Descending")
106 ]
107
108 onSelectedIndexChanged: {
109 fileView.sortAscending = (values[selectedIndex] === i18n.tr("Ascending"))
110 }
111 }
112
113 ListItem.Standard {
114 text: i18n.tr("Show Advanced Features")
115 control: CheckBox {
116 id: showAdvancedFeaturesCheckBox
117 checked: getSetting("showAdvancedFeatures", false);
118 onCheckedChanged: {
119 saveSetting("showAdvancedFeatures",
120 showAdvancedFeaturesCheckBox.checked ? true : false);
121 }
122 }
123 }
124
125 ListItem.Standard {
126 text: i18n.tr("Filter")
127
128 visible: fileView.showAdvancedFeatures
129
130 control: TextField {
131 id: filterField
132 anchors {
133 verticalCenter: parent.verticalCenter
134 right: parent.right
135 margins: units.gu(1)
136 }
137
138 inputMethodHints: Qt.ImhNoAutoUppercase
139
140 text: pageModel.nameFilters
141
142 onAccepted: goButton.clicked()
143 onTextChanged: {
144 if (text !== pageModel.nameFilters)
145 pageModel.nameFilters = [text]
146 }
147 }
148 }
149
150 Component {
151 id: selectorDelegate
152
153 OptionSelectorDelegate { showDivider: false; }
50 }154 }
51 }155 }
52 }156 }
53157
=== removed file 'src/app/qml/ui/ViewPopover.qml'
--- src/app/qml/ui/ViewPopover.qml 2014-09-20 10:49:51 +0000
+++ src/app/qml/ui/ViewPopover.qml 1970-01-01 00:00:00 +0000
@@ -1,155 +0,0 @@
1/*
2 * Copyright (C) 2013 Canonical Ltd
3 *
4 * This program is free software: you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License version 3 as
6 * published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Michael Spencer <sonrisesoftware@gmail.com>
17 */
18import QtQuick 2.3
19import Ubuntu.Components 1.1
20import Ubuntu.Components.Popups 1.0
21import Ubuntu.Components.ListItems 1.0
22
23Popover {
24 id: root
25 objectName: "viewPopover"
26
27 Column {
28 anchors {
29 left: parent.left
30 right: parent.right
31 top: parent.top
32 }
33
34 Standard {
35 id: showHiddenFileCheckBox
36 objectName: "showHiddenFileCheckBox"
37
38 Label {
39 text: i18n.tr("Show Hidden Files")
40 fontSize: "medium"
41 color: Theme.palette.normal.overlayText
42 anchors.left: parent.left
43 anchors.leftMargin: units.gu(2)
44 anchors.verticalCenter: parent.verticalCenter
45 }
46
47 control: CheckBox {
48 anchors.verticalCenter: parent.verticalCenter
49
50 checked: fileView.showHiddenFiles
51 onCheckedChanged: {
52 fileView.showHiddenFiles = checked
53 }
54 }
55 }
56
57 ValueSelector {
58 Label {
59 text: i18n.tr("View As")
60 fontSize: "medium"
61 color: Theme.palette.normal.overlayText
62 anchors.left: parent.left
63 anchors.leftMargin: units.gu(2)
64 anchors.top: parent.top
65 anchors.topMargin: units.gu(1.6)
66 }
67
68 selectedIndex: values.indexOf(viewMethod)
69 values: [
70 i18n.tr("Icons"),
71 i18n.tr("List")
72 ]
73
74 onSelectedIndexChanged: {
75 saveSetting("viewMethod", values[selectedIndex])
76 }
77 }
78
79 ValueSelector {
80 Label {
81 text: i18n.tr("Sort By")
82 fontSize: "medium"
83 color: Theme.palette.normal.overlayText
84 anchors.left: parent.left
85 anchors.leftMargin: units.gu(2)
86 anchors.top: parent.top
87 anchors.topMargin: units.gu(1.6)
88 }
89
90 selectedIndex: values.indexOf(fileView.sortingMethod)
91 values: [
92 i18n.tr("Name"),
93 i18n.tr("Date")
94 ]
95
96 onSelectedIndexChanged: {
97 fileView.sortingMethod = values[selectedIndex]
98 }
99 }
100
101 ValueSelector {
102 Label {
103 text: i18n.tr("Sort Order")
104 fontSize: "medium"
105 color: Theme.palette.normal.overlayText
106 anchors.left: parent.left
107 anchors.leftMargin: units.gu(2)
108 anchors.top: parent.top
109 anchors.topMargin: units.gu(1.7)
110 }
111
112 selectedIndex: sortAscending ? 0 : 1
113 values: [
114 i18n.tr("Ascending"),
115 i18n.tr("Descending")
116 ]
117
118 onSelectedIndexChanged: {
119 fileView.sortAscending = (values[selectedIndex] === i18n.tr("Ascending"))
120 }
121 }
122
123 Standard {
124 visible: showAdvancedFeatures
125
126 Label {
127 text: i18n.tr("Filter")
128 fontSize: "medium"
129 color: Theme.palette.normal.overlayText
130 anchors.left: parent.left
131 anchors.leftMargin: units.gu(2)
132 anchors.verticalCenter: parent.verticalCenter
133 }
134
135 TextField {
136 id: filterField
137 anchors {
138 verticalCenter: parent.verticalCenter
139 right: parent.right
140 margins: units.gu(1)
141 }
142
143 inputMethodHints: Qt.ImhNoAutoUppercase
144
145 text: pageModel.nameFilters
146
147 onAccepted: goButton.clicked()
148 onTextChanged: {
149 if (text !== pageModel.nameFilters)
150 pageModel.nameFilters = [text]
151 }
152 }
153 }
154 }
155}

Subscribers

People subscribed via source and target branches