Merge lp:~zsombi/ubuntu-ui-toolkit/dynamic-tabs into lp:ubuntu-ui-toolkit

Proposed by Zsombor Egri on 2013-10-30
Status: Merged
Approved by: Zoltan Balogh on 2013-11-18
Approved revision: 831
Merged at revision: 836
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/dynamic-tabs
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 710 lines (+468/-46)
8 files modified
components.api (+6/-1)
modules/Ubuntu/Components/Tab.qml (+24/-0)
modules/Ubuntu/Components/TabBar.qml (+11/-0)
modules/Ubuntu/Components/Tabs.qml (+195/-37)
modules/Ubuntu/Components/Themes/Ambiance/TabBarStyle.qml (+5/-4)
tests/resources/navigation/Tabs.qml (+51/-4)
tests/unit_x11/tst_components/ExternalTab.qml (+21/-0)
tests/unit_x11/tst_components/tst_tabs.qml (+155/-0)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/dynamic-tabs
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-11-17
Christian Dywan (community) Approve on 2013-11-13
Tim Peeters 2013-10-30 Approve on 2013-11-11
Zsombor Egri Approve on 2013-11-08
Review via email: mp+193198@code.launchpad.net

Commit message

Support for dynamically inserting/moving/removing tabs in Tabs.

To post a comment you must log in.
Tim Peeters (tpeeters) wrote :

Tab.qml:

    QtObject {
        id: internal
        property int index: -1
        property bool inserted: false
        property bool dynamic: false
        property bool removedFromTabs: false
    }

Can you add a short description to these properties?
What does dynamic mean? Inserted means it is inside a Tabs, or only true if inserted using insert()?

Tim Peeters (tpeeters) wrote :

51 + \li \b tab - optional, the Tab or other Item to be displayed when activated;
52 + if specified the component will be activated when selected; otherwise
53 + the index will be used to activate the item

Other Item is currently not supported. Do we want to complicate things and allow it now?

Tim Peeters (tpeeters) wrote :

    onChildrenChanged: tabStack.updateTabList(children)

huh? tabStack doesn't have that function. tabsModel does. Where are the tests to catch this?

review: Needs Fixing
Tim Peeters (tpeeters) wrote :

Why is there a tabStack? Is it really a stack?

Tim Peeters (tpeeters) wrote :

121 + \a title, unless if the given value is empty or undefined. The optional

empty --> an empty string is a bit more precise.

Tim Peeters (tpeeters) wrote :

130 + Inserts a Tab at the given index. If the \a index is less than 0, the Tab

I would say less or equal, because 0 is the first index.

Tim Peeters (tpeeters) wrote :

132 + is greater than \l count. \a title, \a component and \a params are used in
133 + the same way as in \l addTab(). Returns the instance of the created Tab.

"created" is not correct (same for addTab), because the tab may already exist and just be inserted. What about "inserted tab" and "added tab"?

Tim Peeters (tpeeters) wrote :

150 + } else {
151 + var tabComponent = null;
152 + if (typeof component === "string") {
153 + tabComponent = Qt.createComponent(component);
154 + } else {
155 + tabComponent = component;
156 + }
157 + if (tabComponent.status === Component.Error) {
158 + console.error(tabComponent.errorString());
159 + return null;
160 + }
161 + tab = tabComponent.createObject();
162 + tab.__protected.dynamic = true;
etc

What happens here is similar to what is done in PageWrapperUtils.js.
Could we generalize that and have a single js file for taking care of these kind of things?
No need to do it for this MR; just a thought.

Tim Peeters (tpeeters) wrote :

177 + if (tabs.selectedTabIndex >= index) {
178 + // move the selected index to the next index
179 + tabs.selectedTabIndex += 1;
180 + } else {
181 + tabs.modelChanged();
182 + }

Why is modelChanged inside the else { }? Should it not be outside? The model also changed if selectedTabIndex < index.

Tim Peeters (tpeeters) wrote :

02 + // fix selected tab
203 + if (selectedTabIndex === from) {
204 + selectedTabIndex = to;
205 + } else if (selectedTabIndex <= to && selectedTabIndex >= from) {
206 + selectedTabIndex -= 1;
207 + } else {
208 + tabs.modelChanged();
209 + }
210 +

Same as the comment above. Always execute modelChanged()?

Tim Peeters (tpeeters) wrote :

207 + } else {
208 + tabs.modelChanged();
209 + }

Tim Peeters (tpeeters) wrote :

316 + for (var i in tabStack.children) {

is this a mistake, or did I just learn about a feature of QML?

Tim Peeters (tpeeters) wrote :

315 + function dymanicRemove() {
316 + for (var i in tabStack.children) {
317 + if (this === tabStack.children[i]) {
318 + tabs.removeTab(i);
319 + break;
320 }

That's a bit confusing for me. What is 'this' here?

Tim Peeters (tpeeters) wrote :

411 + Component {
412 + id: dynamicTab
413 + Tab {
414 + title: "Original Title"
415 + page: Page {}
416 + }
417 + }

You could add delete/move buttons here, just for testing.

Tim Peeters (tpeeters) wrote :

awesome stuff :) after answering the questions above and testing on device it can be merged.

Zsombor Egri (zsombi) wrote :

> Tab.qml:
>
> QtObject {
> id: internal
> property int index: -1
> property bool inserted: false
> property bool dynamic: false
> property bool removedFromTabs: false
> }
>
>
> Can you add a short description to these properties?
> What does dynamic mean? Inserted means it is inside a Tabs, or only true if
> inserted using insert()?
Fixed in revno 821

Zsombor Egri (zsombi) wrote :

> 51 + \li \b tab - optional, the Tab or other Item to be displayed when
> activated;
> 52 + if specified the component will be activated when selected;
> otherwise
> 53 + the index will be used to activate the item
>
> Other Item is currently not supported. Do we want to complicate things and
> allow it now?
Nope, you're right. Actually the only role we mandate is title, so I'll remove the tab. Fixed in revno 822.

Zsombor Egri (zsombi) wrote :

> onChildrenChanged: tabStack.updateTabList(children)
>
> huh? tabStack doesn't have that function. tabsModel does. Where are the tests
> to catch this?

Good finding! It's a residuum :( Fixed in revno 823

Zsombor Egri (zsombi) wrote :

> Why is there a tabStack? Is it really a stack?
Well, it's kind of stack, right? Though the components are not shown o top of each other, it is a stack what comes to holding Tab components, so the name is accurate. However it could be tabHolder, whatever, but it is definitely not a model.

Zsombor Egri (zsombi) wrote :

> 121 + \a title, unless if the given value is empty or undefined. The
> optional
>
> empty --> an empty string is a bit more precise.
Fixed in revno 823.

Zsombor Egri (zsombi) wrote :

> 130 + Inserts a Tab at the given index. If the \a index is less than 0,
> the Tab
>
>
> I would say less or equal, because 0 is the first index.
Fixed in revno 825.

Zsombor Egri (zsombi) wrote :

> 132 + is greater than \l count. \a title, \a component and \a params are
> used in
> 133 + the same way as in \l addTab(). Returns the instance of the created
> Tab.
>
>
> "created" is not correct (same for addTab), because the tab may already exist
> and just be inserted. What about "inserted tab" and "added tab"?
Also fixed in 825.

Zsombor Egri (zsombi) wrote :

> 150 + } else {
> 151 + var tabComponent = null;
> 152 + if (typeof component === "string") {
> 153 + tabComponent = Qt.createComponent(component);
> 154 + } else {
> 155 + tabComponent = component;
> 156 + }
> 157 + if (tabComponent.status === Component.Error) {
> 158 + console.error(tabComponent.errorString());
> 159 + return null;
> 160 + }
> 161 + tab = tabComponent.createObject();
> 162 + tab.__protected.dynamic = true;
> etc
>
>
> What happens here is similar to what is done in PageWrapperUtils.js.
> Could we generalize that and have a single js file for taking care of these
> kind of things?
> No need to do it for this MR; just a thought.
Right, we could do that at some point.

Zsombor Egri (zsombi) wrote :

> 177 + if (tabs.selectedTabIndex >= index) {
> 178 + // move the selected index to the next index
> 179 + tabs.selectedTabIndex += 1;
> 180 + } else {
> 181 + tabs.modelChanged();
> 182 + }
>
> Why is modelChanged inside the else { }? Should it not be outside? The model
> also changed if selectedTabIndex < index.
When you change the selectedTabIndex, the visuals are automatically updated and will point to the proper tab. However if the selected tab stays at the same index, but the model content is changed, PathView scrolls the content away and there will be nothing shown in the TabBar. Thus need to refresh the visuals by calling that signal (I'm removing that signal and substing with a sync() function)

Zsombor Egri (zsombi) wrote :

> 316 + for (var i in tabStack.children) {
>
> is this a mistake, or did I just learn about a feature of QML?
I hope you just did :)

Zsombor Egri (zsombi) wrote :

> 315 + function dymanicRemove() {
> 316 + for (var i in tabStack.children) {
> 317 + if (this === tabStack.children[i]) {
> 318 + tabs.removeTab(i);
> 319 + break;
> 320 }
>
>
> That's a bit confusing for me. What is 'this' here?
'this' is the caller object, to which the function was bent to. However this is not needed as we assume tabs are either pre-declared or created by the Tabs functions. So I removed the relevant functionality in revno 826.

Zsombor Egri (zsombi) wrote :

> 411 + Component {
> 412 + id: dynamicTab
> 413 + Tab {
> 414 + title: "Original Title"
> 415 + page: Page {}
> 416 + }
> 417 + }
>
> You could add delete/move buttons here, just for testing.
I've added a bit more to the sample to also reflect indexes for each tab. Revno 827.

Zsombor Egri (zsombi) wrote :

> 51 + \li \b tab - optional, the Tab or other Item to be displayed when
> activated;
> 52 + if specified the component will be activated when selected;
> otherwise
> 53 + the index will be used to activate the item
>
> Other Item is currently not supported. Do we want to complicate things and
> allow it now?
Actually I've added the tab role back, and added a small detail about how these roles are used in revno 828.

PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:828
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1176/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/547
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/535
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/124
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/124
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/124/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/504
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/547
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/547/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/535
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/535/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/3002
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3184
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/1188
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/1187

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/1176/rebuild

review: Approve (continuous-integration)
Zsombor Egri (zsombi) wrote :

Tested on Galaxy Nexus with ubuntuuitoolkit and gallery-app AP tests.

Tim Peeters (tpeeters) wrote :

Good to go :)

Just waiting for the AP 1.4 tests in the new image to pass so we can merge our stuff again.

review: Approve
Zsombor Egri (zsombi) wrote :

HOLD! There are some AP tests failing, need to make sure it is not caused by this MR!

review: Needs Fixing
Zsombor Egri (zsombi) wrote :

All passing. Can go in once we get a green light.

review: Approve
Tim Peeters (tpeeters) wrote :

All good. Just do another trunk merge to get Friday's bug fixes and if there are no conflicts, find someone to happrove.

review: Approve
Tim Peeters (tpeeters) wrote :

I reported this bug for the TabBar: https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1250194

It doesn't have to be fixed in this MR, but it is related. Let's see if we can fix it in a separate MR.

Christian Dywan (kalikiana) wrote :

646 + // add a new tab anc check the count (default added tas should not be added anymore

Typo

I don't see a unit test for loading a tab from a URL.

review: Needs Fixing
Zsombor Egri (zsombi) wrote :

> 646 + // add a new tab anc check the count (default added tas
> should not be added anymore
>
> Typo
>
> I don't see a unit test for loading a tab from a URL.

Fixed in revno 831.

Christian Dywan (kalikiana) wrote :

Nice stuff! Thanks for adding the tests!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components.api'
2--- components.api 2013-11-08 20:31:45 +0000
3+++ components.api 2013-11-13 10:18:46 +0000
4@@ -207,6 +207,7 @@
5 modules/Ubuntu/Components/TabBar.qml
6 StyledItem
7 property Item tabsItem
8+ property ListModel model
9 property bool selectionMode
10 property bool alwaysSelectionMode
11 property bool animate
12@@ -216,9 +217,13 @@
13 readonly property Tab selectedTab
14 readonly property Item currentPage
15 property TabBar tabBar
16- property internal __tabs
17 default property list<Item> tabChildren
18+ readonly property int count
19 signal modelChanged()
20+ function addTab(title, component, params)
21+ function insertTab(index, title, component, params)
22+ function moveTab(from, to)
23+ function removeTab(index)
24 modules/Ubuntu/Components/TextArea.qml
25 StyledItem
26 property bool highlighted
27
28=== modified file 'modules/Ubuntu/Components/Tab.qml'
29--- modules/Ubuntu/Components/Tab.qml 2013-10-02 06:23:44 +0000
30+++ modules/Ubuntu/Components/Tab.qml 2013-11-13 10:18:46 +0000
31@@ -87,6 +87,30 @@
32 property alias __protected: internal
33 QtObject {
34 id: internal
35+ /*
36+ Specifies the index of the Tab in Tabs.
37+ */
38 property int index: -1
39+
40+ /*
41+ Specifies whether the Tab has already been inserted in Tabs model or not.
42+ Pre-declared tabs are added one by one automatically before Tabs component
43+ completion, therefore we need this flag to exclude adding those Tab elements
44+ again which were already added.
45+ */
46+ property bool inserted: false
47+
48+ /*
49+ Specifies whether the Tab was created dynamically or not. A dynamically created
50+ Tab is destroyed upon removal.
51+ */
52+ property bool dynamic: false
53+
54+ /*
55+ This flag is used by the Tabs to determine whether the pre-declared Tab was removed
56+ from the Tabs model or not. The flag guards adding back pre-declared tabs upon Tabs
57+ component stack (children) change.
58+ */
59+ property bool removedFromTabs: false
60 }
61 }
62
63=== modified file 'modules/Ubuntu/Components/TabBar.qml'
64--- modules/Ubuntu/Components/TabBar.qml 2013-11-04 15:23:45 +0000
65+++ modules/Ubuntu/Components/TabBar.qml 2013-11-13 10:18:46 +0000
66@@ -40,6 +40,17 @@
67 property Item tabsItem
68
69 /*!
70+ The model containing the tabs to be controlled by the TabBar. The tabs are
71+ visualized by the style, displaying controlling elements based on the data
72+ specified by the roles. The default style mandates the existence of either
73+ the \b title or \b tab role, but different styles may require to have other
74+ roles (e.g. image, color). The order the role existence is checked is also
75+ determined by the style component, Default style checks the existence of the
76+ \b tab role first, and if not defined will use the \b title role.
77+ */
78+ property ListModel model
79+
80+ /*!
81 An inactive tab bar only displays the currently selected tab,
82 and an active tab bar can be interacted with to select a tab.
83 */
84
85=== modified file 'modules/Ubuntu/Components/Tabs.qml'
86--- modules/Ubuntu/Components/Tabs.qml 2013-10-02 06:23:44 +0000
87+++ modules/Ubuntu/Components/Tabs.qml 2013-11-13 10:18:46 +0000
88@@ -155,14 +155,14 @@
89 The first tab is 0, and -1 means that no tab is selected.
90 The initial value is 0 if Tabs has contents, or -1 otherwise.
91 */
92- property int selectedTabIndex: tabs.__tabs.length > 0 ? 0 : -1
93+ property int selectedTabIndex: tabsModel.count > 0 ? 0 : -1
94
95 /*!
96 \preliminary
97 The currently selected tab.
98 */
99- readonly property Tab selectedTab: (selectedTabIndex < 0) || (__tabs.length <= selectedTabIndex) ?
100- null : __tabs[selectedTabIndex]
101+ readonly property Tab selectedTab: (selectedTabIndex < 0) || (tabsModel.count <= selectedTabIndex) ?
102+ null : tabsModel.get(selectedTabIndex).tab
103
104 /*!
105 The page of the currently selected tab.
106@@ -175,20 +175,21 @@
107 */
108 property TabBar tabBar: TabBar {
109 tabsItem: tabs
110+ model: tabsModel
111 visible: tabs.active
112 }
113
114 /*!
115- \internal
116- Used by the style to create the tabs header.
117- */
118- property alias __tabs: tabsModel.tabList
119-
120- /*!
121 Children are placed in a separate item that has functionality to extract the Tab items.
122 \qmlproperty list<Item> tabChildren
123 */
124- default property alias tabChildren: tabsModel.children
125+ default property alias tabChildren: tabStack.data
126+
127+ /*!
128+ \qmlproperty int count
129+ Contains the number of tabs in the Tabs component.
130+ */
131+ readonly property alias count: tabsModel.count
132
133 /*!
134 Used by the tabs style to update the tabs header with the titles of all the tabs.
135@@ -198,30 +199,195 @@
136 signal modelChanged()
137
138 /*!
139+ Appends a Tab dynamically to the list of tabs. The \a title specifies the
140+ title of the Tab. The \a component can be either a Component, a URL to
141+ the Tab component to be loaded or an instance of a pre-declared tab that
142+ has been previously removed. The Tab's title will be replaced with the given
143+ \a title, unless if the given value is empty string or undefined. The optional
144+ \a params defines parameters passed to the Tab.
145+ Returns the instance of the added Tab.
146+ */
147+ function addTab(title, component, params) {
148+ return insertTab(count, title, component, params);
149+ }
150+
151+ /*!
152+ Inserts a Tab at the given index. If the \a index is less or equal than 0,
153+ the Tab will be added to the front, and to the end of the tab stack if the
154+ \a index is greater than \l count. \a title, \a component and \a params
155+ are used in the same way as in \l addTab(). Returns the instance of the
156+ inserted Tab.
157+ */
158+ function insertTab(index, title, component, params) {
159+ // check if the given component is a Tab instance
160+ var tab = null;
161+ if (component && component.hasOwnProperty("page") && component.hasOwnProperty("__protected")) {
162+ // dynamically added components are destroyed upon removal, so
163+ // in case we get a Tab as parameter, we can only have a predeclared one
164+ // therefore we simply restore the default state of the removedFromTabs property
165+ // and return the instance
166+ if (!component.__protected.removedFromTabs) {
167+ // exit if the Tab is not removed
168+ return null;
169+ }
170+
171+ component.__protected.removedFromTabs = false;
172+ tab = component;
173+ } else {
174+ var tabComponent = null;
175+ if (typeof component === "string") {
176+ tabComponent = Qt.createComponent(component);
177+ } else {
178+ tabComponent = component;
179+ }
180+ if (tabComponent.status === Component.Error) {
181+ console.error(tabComponent.errorString());
182+ return null;
183+ }
184+ tab = tabComponent.createObject();
185+ tab.__protected.dynamic = true;
186+ }
187+
188+ // fix title
189+ if (title !== undefined && title !== "") {
190+ tab.title = title;
191+ }
192+
193+ // insert the created tab into the model
194+ index = MathUtils.clamp(index, 0, count);
195+ tab.__protected.inserted = true;
196+ tab.__protected.index = index;
197+ tabsModel.insert(index, tabsModel.listModel(tab));
198+ tabsModel.reindex(index);
199+ tab.parent = tabStack;
200+ if (tabs.selectedTabIndex >= index) {
201+ // move the selected index to the next index
202+ tabs.selectedTabIndex += 1;
203+ } else {
204+ tabs.modelChanged();
205+ }
206+ return tab;
207+ }
208+
209+ /*!
210+ Moves the tab from the given \a from position to the position given in \a to.
211+ Returns true if the indexes were in 0..\l count - 1 boundary and if the operation
212+ succeeds, and false otherwise. The \l selectedTabIndex is updated if it is
213+ affected by the move (it is equal with \a from or falls between \a from and
214+ \a to indexes).
215+ */
216+ function moveTab(from, to) {
217+ if (from < 0 || from >= count || to < 0 || to >= count || from === to) return false;
218+ var tabFrom = tabsModel.get(from).tab;
219+ var tabTo = tabsModel.get(to).tab;
220+
221+ // move tab
222+ tabsModel.move(from, to, 1);
223+ tabsModel.reindex();
224+
225+ // fix selected tab
226+ if (selectedTabIndex === from) {
227+ selectedTabIndex = to;
228+ } else if (selectedTabIndex <= to && selectedTabIndex >= from) {
229+ selectedTabIndex -= 1;
230+ } else {
231+ tabs.modelChanged();
232+ }
233+
234+ return true;
235+ }
236+
237+ /*!
238+ Removes the Tab from the given \a index. Returns true if the \a index falls
239+ into 0..\l count - 1 boundary and the operation succeeds, and false on error.
240+ The function removes also the pre-declared tabs. These can be added back using
241+ \l addTab or \l insertTab by specifying the instance of the Tab to be added as
242+ component. The \l selectedTabIndex is updated if is affected by the removal
243+ (it is identical or greater than the tab index to be removed).
244+ */
245+ function removeTab(index) {
246+ if (index < 0 || index >= count) return false;
247+ var tab = tabsModel.get(index).tab;
248+ var activeIndex = (selectedTabIndex >= index) ? MathUtils.clamp(selectedTabIndex, 0, count - 2) : -1;
249+
250+ tabsModel.remove(index);
251+ tabsModel.reindex();
252+ // move active tab if needed
253+ if (activeIndex >= 0) {
254+ selectedTabIndex = activeIndex;
255+ }
256+
257+ if (tab.__protected.dynamic) {
258+ tab.destroy();
259+ } else {
260+ // pre-declared tab, mark it as removed, so we don't update it next time
261+ // the tabs stack children is updated
262+ tab.__protected.removedFromTabs = true;
263+ }
264+
265+ if (activeIndex < 0) {
266+ tabs.modelChanged();
267+ }
268+ return true;
269+ }
270+
271+ /*!
272 \internal
273 required by TabsStyle
274 */
275+ ListModel {
276+ id: tabsModel
277+
278+ function listModel(tab) {
279+ return {"title": tab.title, "tab": tab};
280+ }
281+
282+ function updateTabList(tabsList) {
283+ for (var i in tabsList) {
284+ var tab = tabsList[i];
285+ if (internal.isTab(tab)) {
286+ // make sure we have the right parent
287+ tab.parent = tabStack;
288+
289+ if (!tab.__protected.inserted) {
290+ tab.__protected.index = count;
291+ tab.__protected.inserted = true;
292+ append(listModel(tab));
293+ } else if (!tab.__protected.removedFromTabs && tabsModel.count > tab.index) {
294+ get(tab.index).title = tab.title;
295+ }
296+ }
297+ }
298+ tabs.modelChanged();
299+ }
300+
301+ function reindex(from) {
302+ var start = 0;
303+ if (from !== undefined) {
304+ start = from + 1;
305+ }
306+
307+ for (var i = start; i < count; i++) {
308+ var tab = get(i).tab;
309+ tab.__protected.index = i;
310+ }
311+ }
312+
313+ }
314+
315 Item {
316 anchors.fill: parent
317- id: tabsModel
318-
319- property var tabList: []
320- onChildrenChanged: {
321- updateTabList();
322- }
323-
324- function updateTabList() {
325- var list = [];
326- var index = 0;
327- for (var i=0; i < children.length; i++) {
328- if (isTab(tabsModel.children[i])) {
329- tabsModel.children[i].__protected.index = index++;
330- list.push(tabsModel.children[i]);
331- }
332- }
333- tabList = list;
334- tabs.modelChanged();
335- }
336+ id: tabStack
337+
338+ onChildrenChanged: tabsModel.updateTabList(children)
339+ }
340+
341+ /*! \internal */
342+ onModelChanged: if (tabs.active && internal.header) internal.header.show()
343+
344+ QtObject {
345+ id: internal
346+ property Header header: tabs.__propagated ? tabs.__propagated.header : null
347
348 function isTab(item) {
349 if (item && item.hasOwnProperty("__isPageTreeNode")
350@@ -234,14 +400,6 @@
351 }
352 }
353
354- /*! \internal */
355- onModelChanged: if (tabs.active && internal.header) internal.header.show()
356-
357- QtObject {
358- id: internal
359- property Header header: tabs.__propagated ? tabs.__propagated.header : null
360- }
361-
362 Binding {
363 target: internal.header
364 property: "contents"
365
366=== modified file 'modules/Ubuntu/Components/Themes/Ambiance/TabBarStyle.qml'
367--- modules/Ubuntu/Components/Themes/Ambiance/TabBarStyle.qml 2013-10-29 19:00:37 +0000
368+++ modules/Ubuntu/Components/Themes/Ambiance/TabBarStyle.qml 2013-11-13 10:18:46 +0000
369@@ -41,6 +41,7 @@
370 The set of tabs this tab bar belongs to
371 */
372 property Tabs tabs: styledItem ? styledItem.tabsItem : null
373+ property ListModel tabsModel : styledItem ? styledItem.model : null
374
375 Connections {
376 target: styledItem
377@@ -90,7 +91,7 @@
378
379 Repeater {
380 id: repeater
381- model: tabs.__tabs
382+ model: tabsModel
383
384 AbstractButton {
385 id: button
386@@ -123,7 +124,7 @@
387
388 // When we don't need scrolling, we want to avoid showing a button that is fading
389 // while sliding in from the right side when a new button was selected
390- var numTabs = tabs.__tabs.length;
391+ var numTabs = tabsModel.count;
392 var minimum = buttonView.selectedButtonIndex;
393 var maximum = buttonView.selectedButtonIndex + numTabs - 1;
394 if (MathUtils.clamp(buttonIndex, minimum, maximum) === buttonIndex) return true;
395@@ -179,7 +180,7 @@
396 baseline: parent.bottom
397 baselineOffset: -headerTextBottomMargin
398 }
399- text: modelData.title
400+ text: (tab && tab.hasOwnProperty("title")) ? tab.title : title
401 fontSize: headerFontSize
402 font.weight: headerFontWeight
403 }
404@@ -254,7 +255,7 @@
405
406 // Select the closest of the two buttons that represent the given tab index
407 function selectButton(tabIndex) {
408- if (tabIndex < 0 || tabIndex >= tabs.__tabs.length) return;
409+ if (tabIndex < 0 || tabIndex >= tabsModel.count) return;
410 if (buttonView.buttonRow1 && buttonView.buttonRow2) {
411 var b1 = buttonView.buttonRow1.children[tabIndex];
412 var b2 = buttonView.buttonRow2.children[tabIndex];
413
414=== modified file 'tests/resources/navigation/Tabs.qml'
415--- tests/resources/navigation/Tabs.qml 2013-08-01 13:16:42 +0000
416+++ tests/resources/navigation/Tabs.qml 2013-11-13 10:18:46 +0000
417@@ -21,6 +21,28 @@
418 MainView {
419 width: 800
420 height: 600
421+
422+ Component {
423+ id: dynamicTab
424+ Tab {
425+ title: "Original Title #" + index
426+ page: Page {
427+ Row {
428+ anchors.centerIn: parent
429+ spacing: 5
430+ Button {
431+ text: "Delete"
432+ onClicked: tabs.removeTab(index)
433+ }
434+ Button {
435+ text: "Move to 0"
436+ onClicked: tabs.moveTab(index, 0)
437+ }
438+ }
439+ }
440+ }
441+ }
442+
443 Tabs {
444 id: tabs
445 selectedTabIndex: 0
446@@ -29,7 +51,8 @@
447 }
448
449 Tab {
450- title: i18n.tr("Simple page")
451+ id: simpleTab
452+ title: i18n.tr("Simple page #" + index)
453 page: Page {
454 title: "This title is not visible"
455 Row {
456@@ -54,13 +77,37 @@
457 iconSource: "call_icon.png"
458 onTriggered: print("action triggered")
459 }
460+ ToolbarButton {
461+ text: "ADD"
462+ iconSource: "call_icon.png"
463+ onTriggered: tabs.addTab("", dynamicTab)
464+ }
465+ ToolbarButton {
466+ text: "INSERT"
467+ iconSource: "call_icon.png"
468+ onTriggered: tabs.insertTab(1, "INSERTED", dynamicTab)
469+ }
470+ ToolbarButton {
471+ text: "move me next"
472+ iconSource: "call_icon.png"
473+ onTriggered: {
474+ var i = simpleTab.index;
475+ tabs.moveTab(i, i + 1);
476+ }
477+ }
478+ ToolbarButton {
479+ text: "delete 0"
480+ iconSource: "call_icon.png"
481+ onTriggered: tabs.removeTab(0)
482+ }
483 }
484 }
485 }
486 Repeater {
487 model: 3
488 Tab {
489- title: "Extra " + index
490+ id: tab
491+ title: "Extra #" + tab.index
492 page: Page {
493 Column {
494 anchors.centerIn: parent
495@@ -86,7 +133,7 @@
496 }
497 Tab {
498 id: externalTab
499- title: i18n.tr("External")
500+ title: i18n.tr("External #" + index)
501 page: Loader {
502 parent: externalTab
503 anchors.fill: parent
504@@ -94,7 +141,7 @@
505 }
506 }
507 Tab {
508- title: i18n.tr("List view")
509+ title: i18n.tr("List view #" + index)
510 page: Page {
511 ListView {
512 clip: true
513
514=== added file 'tests/unit_x11/tst_components/ExternalTab.qml'
515--- tests/unit_x11/tst_components/ExternalTab.qml 1970-01-01 00:00:00 +0000
516+++ tests/unit_x11/tst_components/ExternalTab.qml 2013-11-13 10:18:46 +0000
517@@ -0,0 +1,21 @@
518+/*
519+ * Copyright 2012 Canonical Ltd.
520+ *
521+ * This program is free software; you can redistribute it and/or modify
522+ * it under the terms of the GNU Lesser General Public License as published by
523+ * the Free Software Foundation; version 3.
524+ *
525+ * This program is distributed in the hope that it will be useful,
526+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
527+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
528+ * GNU Lesser General Public License for more details.
529+ *
530+ * You should have received a copy of the GNU Lesser General Public License
531+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
532+ */
533+
534+import QtQuick 2.0
535+import Ubuntu.Components 0.1
536+
537+Tab {
538+}
539
540=== modified file 'tests/unit_x11/tst_components/tst_tabs.qml'
541--- tests/unit_x11/tst_components/tst_tabs.qml 2013-11-08 20:36:16 +0000
542+++ tests/unit_x11/tst_components/tst_tabs.qml 2013-11-13 10:18:46 +0000
543@@ -26,6 +26,13 @@
544 id: emptyTabs
545 }
546
547+ Component {
548+ id: dynamicTab
549+ Tab{
550+ title: "OriginalTitle"
551+ }
552+ }
553+
554 MainView {
555 id: mainView
556 anchors.fill: parent
557@@ -184,5 +191,153 @@
558 mouseClick(button, units.gu(1), units.gu(1), Qt.LeftButton);
559 compare(tabs.tabBar.selectionMode, false, "Tab bar deactivated by interacting with the page contents");
560 }
561+
562+ function test_z_addTab() {
563+ var newTab = tabs.addTab("Dynamic Tab", dynamicTab);
564+ compare((newTab !== null), true, "tab added");
565+ compare(newTab.active, false, "the inserted tab is inactive");
566+ compare(newTab.index, tabs.count - 1, "the tab is the last one");
567+ }
568+
569+ function test_z_addExternalTab() {
570+ var newTab = tabs.addTab("External Tab", Qt.resolvedUrl("ExternalTab.qml"));
571+ compare((newTab !== null), true, "tab added");
572+ compare(newTab.active, false, "the inserted tab is inactive");
573+ compare(newTab.index, tabs.count - 1, "the tab is the last one");
574+ }
575+
576+ function test_z_addTabWithDefaultTitle() {
577+ var newTab = tabs.addTab("", dynamicTab);
578+ compare((newTab !== null), true, "tab added");
579+ compare(newTab.title, "OriginalTitle", "tab created with original title");
580+ }
581+
582+ function test_z_insertTab() {
583+ var tabIndex = Math.ceil(tabs.count / 2);
584+ var newTab = tabs.insertTab(tabIndex, "Inserted tab", dynamicTab);
585+ compare((newTab !== null), true, "tab inserted");
586+ compare(newTab.index, tabIndex, "this is the first tab");
587+ compare(tabs.selectedTab !== newTab, true, "the new tab is not the active one");
588+ }
589+
590+ function test_z_insertExternalTab() {
591+ var tabIndex = Math.ceil(tabs.count / 2);
592+ var newTab = tabs.insertTab(tabIndex, "Inserted External tab", Qt.resolvedUrl("ExternalTab.qml"));
593+ compare((newTab !== null), true, "tab inserted");
594+ compare(newTab.index, tabIndex, "this is the first tab");
595+ compare(tabs.selectedTab !== newTab, true, "the new tab is not the active one");
596+ }
597+
598+ function test_z_insertTabAtSelectedIndex() {
599+ tabs.selectedTabIndex = 1;
600+ var tabIndex = tabs.selectedTabIndex - 1;
601+ var newTab = tabs.insertTab(tabIndex, "InsertedAtSelected tab", dynamicTab);
602+ compare((newTab !== null), true, "tab inserted");
603+ compare(newTab.index, tabIndex, "inserted at selected tab");
604+ compare(tabs.selectedTabIndex != (tabIndex + 1), true, "it is not the selected tab");
605+ }
606+
607+ function test_z_insertTabFront() {
608+ var newTab = tabs.insertTab(-1, "PreTab", dynamicTab);
609+ compare(newTab !== null, true, "pre-tab inserted");
610+ compare(newTab.index, 0, "this is the new first tab");
611+ compare(tabs.selectedTab !== newTab, true, "the new tab is not the active one");
612+ }
613+
614+ function test_z_insertTabEnd() {
615+ var newTab = tabs.insertTab(tabs.count, "PostTab", dynamicTab);
616+ compare(newTab !== null, true, "post-tab inserted");
617+ compare(newTab.index, tabs.count - 1, "thsi is the new last tab");
618+ compare(tabs.selectedTab !== newTab, true, "the new tab is not the active one");
619+ }
620+
621+ function test_z_insertTabAndActivate() {
622+ var newTab = tabs.addTab("Inserted tab", dynamicTab);
623+ compare((newTab !== null), true, "tab inserted");
624+ compare(newTab.index, tabs.count - 1, "the tab is the last one");
625+ tabs.selectedTabIndex = newTab.index;
626+ compare(tabs.selectedTab, newTab, "the inserted tab is selected");
627+ compare(newTab.active, true, "the new tab is active");
628+ }
629+
630+ function test_z_moveTab() {
631+ var selectedIndex = tabs.count - 1;
632+ tabs.selectedTabIndex = selectedIndex;
633+ compare(tabs.moveTab(0, selectedIndex), true, "first tab moved to last");
634+ compare(tabs.selectedTabIndex, selectedIndex - 1, "the selected index moved backwards");
635+ tabs.selectedTabIndex = selectedIndex = 0;
636+ compare(tabs.moveTab(selectedIndex, selectedIndex + 1), true, "selected tab moved as next");
637+ compare(tabs.selectedTabIndex, selectedIndex + 1, "the selected index moved forewards");
638+ }
639+
640+ function test_z_moveSelectedTab() {
641+ tabs.selectedTabIndex = 0;
642+ tabs.moveTab(0, 1);
643+ compare(tabs.selectedTabIndex, 1, "selected tab moved");
644+ }
645+
646+ function test_z_moveTabFail() {
647+ compare(tabs.moveTab(-1, tabs.count - 1), false, "from-parameter out of range");
648+ compare(tabs.moveTab(0, tabs.count), false, "to-parameter out of range");
649+ }
650+
651+ function test_z_removeTab() {
652+ compare(tabs.removeTab(tabs.count - 1), true, "last tab removed");
653+ tabs.selectedTabIndex = 0;
654+ compare(tabs.removeTab(0), true, "active tab removed");
655+ compare(tabs.selectedTabIndex, 0, "the next tab is selected")
656+ }
657+
658+ function test_z_removeTabAfterActiveTab() {
659+ var activeTab = tabs.count - 2;
660+ tabs.selectedTabIndex = activeTab;
661+ compare(tabs.removeTab(tabs.count - 1), true, "last tab removed");
662+ compare(tabs.selectedTabIndex, activeTab, "the selected tab wasn't moved");
663+ }
664+
665+ function test_z_removeTabBeforeActiveTab() {
666+ var activeTab = tabs.count - 1;
667+ tabs.selectedTabIndex = activeTab;
668+ compare(tabs.removeTab(0), true, "first tab removed");
669+ compare(tabs.selectedTabIndex, activeTab - 1, "the selected tab index decreased");
670+ }
671+
672+ function test_z_removeActiveTab() {
673+ tabs.selectedTabIndex = 1;
674+ compare(tabs.removeTab(1), true, "selected tab removed");
675+ compare(tabs.selectedTabIndex, 1, "selected tab is next");
676+
677+ tabs.selectedTabIndex = tabs.count - 1;
678+ compare(tabs.removeTab(tabs.count - 1), true, "last tab removed");
679+ compare(tabs.selectedTabIndex, tabs.count - 1, "selected tab moved to last item");
680+ }
681+
682+ function test_zz_addTabAfterCleaningUpTabs() {
683+ while (tabs.count > 1) {
684+ tabs.removeTab(0);
685+ }
686+ compare(tabs.selectedTabIndex, 0, "the only tab is the selected one");
687+ // add a new tab anc check the count (default added tas should not be added anymore
688+ tabs.addTab("Second tab", dynamicTab);
689+ compare(tabs.count, 2, "we have two tabs only");
690+ }
691+
692+ function test_zz_addPredeclaredTab() {
693+ while (tabs.count > 1) {
694+ tabs.removeTab(0);
695+ }
696+ compare(tabs.selectedTabIndex, 0, "the only tab is the selected one");
697+
698+ // add a predeclared tab back with original title
699+ compare(tabs.addTab("", tab1), tab1, "tab1 was added back");
700+ compare(tab1.title, "tab 1", "with the original title");
701+
702+ // add a predeclared tab back with new title
703+ compare(tabs.addTab("Original tab", tab2), tab2, "tab2 was added back");
704+ compare(tab2.title, "Original tab", "with modified title");
705+
706+ // add a predeclared tab which was added already
707+ compare(tabs.addTab("", tab1), null, "tab1 is already in tabs");
708+ }
709 }
710 }

Subscribers

People subscribed via source and target branches

to status/vote changes: