Merge lp:~zsombi/ubuntu-ui-toolkit/dynamic-tabs into lp:ubuntu-ui-toolkit
- dynamic-tabs
- Merge into trunk
| Status: | Merged |
|---|---|
| Approved by: | Zoltan Balogh |
| 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 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| PS Jenkins bot | continuous-integration | Approve | |
| Cris Dywan | Approve | ||
| Tim Peeters | Approve | ||
| Zsombor Egri | Approve | ||
| Review via email: | |||
Commit message
Support for dynamically inserting/
Description of the change
| PS Jenkins bot (ps-jenkins) wrote : | # |
| 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 : | # |
onChildrenC
huh? tabStack doesn't have that function. tabsModel does. Where are the tests to catch this?
| 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.createCompon
154 + } else {
155 + tabComponent = component;
156 + }
157 + if (tabComponent.
158 + console.
159 + return null;
160 + }
161 + tab = tabComponent.
162 + tab.__protected
etc
What happens here is similar to what is done in PageWrapperUtil
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.selectedT
178 + // move the selected index to the next index
179 + tabs.selectedTa
180 + } else {
181 + tabs.modelChang
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.modelChang
209 + }
210 +
Same as the comment above. Always execute modelChanged()?
| Tim Peeters (tpeeters) wrote : | # |
207 + } else {
208 + tabs.modelChang
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.
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.
>
> 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.createCompon
> 154 + } else {
> 155 + tabComponent = component;
> 156 + }
> 157 + if (tabComponent.
> 158 + console.
> 159 + return null;
> 160 + }
> 161 + tab = tabComponent.
> 162 + tab.__protected
> etc
>
>
> What happens here is similar to what is done in PageWrapperUtil
> 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.selectedT
> 178 + // move the selected index to the next index
> 179 + tabs.selectedTa
> 180 + } else {
> 181 + tabs.modelChang
> 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.
> 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://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| 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.
| Zsombor Egri (zsombi) wrote : | # |
HOLD! There are some AP tests failing, need to make sure it is not caused by this MR!
| Zsombor Egri (zsombi) wrote : | # |
All passing. Can go in once we get a green light.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:829
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| 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.
| Tim Peeters (tpeeters) wrote : | # |
I reported this bug for the TabBar: https:/
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.
| Cris 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.
| 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.
| Cris Dywan (kalikiana) wrote : | # |
Nice stuff! Thanks for adding the tests!
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:831
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
| 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 | } |
PASSED: Continuous integration, rev:820
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://