ubuntu-touch-coreapps-reviewers team mailing list archive

[Riccardo Padovani <riccardo@xxxxxxxxxxxxx>]

Review: Needs Fixing

Why did you keep the accounts action button in the notebooks page (and related code)?

Diff comments:

> === modified file 'src/app/qml/reminders.qml'
> --- src/app/qml/reminders.qml	2015-02-24 18:33:04 +0000
> +++ src/app/qml/reminders.qml	2015-02-28 01:31:24 +0000
> @@ -104,7 +104,7 @@
>              accountPage.destroy(100)
>          }
>          var component = Qt.createComponent(Qt.resolvedUrl("ui/AccountSelectorPage.qml"));
> -        accountPage = component.createObject(root, { accounts: accounts, isChangingAccount: isChangingAccount, unauthorizedAccounts: unauthorizedAccounts });
> +        accountPage = component.createObject(root, { accounts: accounts, showBackButton: isChangingAccount, unauthorizedAccounts: unauthorizedAccounts });

Mhh, now the account page is in a tab, why don't you drop all that? Also the accounts action in src/app/qml/ui/NotebooksPage.qml

>          accountPage.accountSelected.connect(function(username, handle) { accountService.startAuthentication(username, handle); pagestack.pop(); root.accountPage = null });
>          pagestack.push(accountPage);
>      }
> @@ -534,6 +534,19 @@
>                      }
>                  }
>              }
> +            Tab {
> +                title: i18n.tr("Accounts")
> +
> +                page: AccountSelectorPage {
> +                    accounts: accounts
> +                    showBackButton: false
> +                    unauthorizedAccounts: true
> +                    onAccountSelected: {
> +                        accountService.startAuthentication(username, handle)
> +                        rootTabs.selectedTabIndex = 0;
> +                    }
> +                }
> +            }
>          }
>      }
>  
> 
> === modified file 'src/app/qml/ui/AccountSelectorPage.qml'
> --- src/app/qml/ui/AccountSelectorPage.qml	2014-12-09 22:43:00 +0000
> +++ src/app/qml/ui/AccountSelectorPage.qml	2015-02-28 01:31:24 +0000
> @@ -29,7 +29,7 @@
>      title: i18n.tr("Select account")
>  
>      property alias accounts: optionSelector.model
> -    property bool isChangingAccount
> +    property bool showBackButton: false
>      property bool unauthorizedAccounts
>  
>      signal accountSelected(string username, var handle)
> @@ -100,7 +100,7 @@
>       }
>  
>       head.backAction: Action {
> -         visible: isChangingAccount
> +         visible: root.showBackButton
>           iconName: "back"
>           text: i18n.tr("Back")
>           onTriggered: { pagestack.pop(); }
> 
> === modified file 'src/app/qml/ui/NotebooksPage.qml'
> --- src/app/qml/ui/NotebooksPage.qml	2014-12-14 22:31:00 +0000
> +++ src/app/qml/ui/NotebooksPage.qml	2015-02-28 01:31:24 +0000
> @@ -60,16 +60,6 @@
>  
>          ToolbarButton {
>              action: Action {
> -                text: i18n.tr("Refresh")
> -                iconName: "reload"
> -                onTriggered: {
> -                    NotesStore.refreshNotebooks();
> -                }
> -            }
> -        }
> -
> -        ToolbarButton {
> -            action: Action {
>                  text: i18n.tr("Accounts")
>                  iconName: "contacts-app-symbolic"
>                  visible: allAccounts.count > 1
> 
> === modified file 'src/app/qml/ui/NotesPage.qml'
> --- src/app/qml/ui/NotesPage.qml	2015-02-25 22:48:18 +0000
> +++ src/app/qml/ui/NotesPage.qml	2015-02-28 01:31:24 +0000
> @@ -71,16 +71,6 @@
>  
>          ToolbarButton {
>              action: Action {
> -                text: i18n.tr("Search")
> -                iconName: "search"
> -                onTriggered: {
> -                    root.openSearch();
> -                }
> -            }
> -        }
> -
> -        ToolbarButton {
> -            action: Action {
>                  iconSource: "../images/sorting.svg"
>                  text: i18n.tr("Sorting")
>                  onTriggered: {
> @@ -97,10 +87,10 @@
>  
>          ToolbarButton {
>              action: Action {
> -                text: i18n.tr("Accounts")
> -                iconName: "contacts-app-symbolic"
> +                text: i18n.tr("Search")
> +                iconName: "search"
>                  onTriggered: {
> -                    openAccountPage(true);
> +                    root.openSearch();
>                  }
>              }
>          }
> 
> === modified file 'src/app/qml/ui/TagsPage.qml'
> --- src/app/qml/ui/TagsPage.qml	2014-12-14 22:31:00 +0000
> +++ src/app/qml/ui/TagsPage.qml	2015-02-28 01:31:24 +0000
> @@ -54,28 +54,6 @@
>                  }
>              }
>          }
> -
> -        ToolbarButton {
> -            action: Action {
> -                text: i18n.tr("Refresh")
> -                iconName: "reload"
> -                onTriggered: {
> -                    NotesStore.refreshNotes();
> -                    tags.refresh();
> -                }
> -            }
> -        }
> -
> -        ToolbarButton {
> -            action: Action {
> -                text: i18n.tr("Accounts")
> -                iconName: "contacts-app-symbolic"
> -                visible: accounts.count > 1
> -                onTriggered: {
> -                    openAccountPage(true);
> -                }
> -            }
> -        }
>      }
>  
>      Tags {
> 


-- 
https://code.launchpad.net/~mzanetti/reminders-app/cleanup-toolbars/+merge/251347
Your team Ubuntu Reminders app developers is subscribed to branch lp:reminders-app.