← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~majster-pl/ubuntu-calendar-app/new-event-page into lp:ubuntu-calendar-app

 

Thanks for review, I added few comments.
Will push fixed code soon.

Diff comments:

> 
> === modified file 'EventRepetition.qml'
> --- EventRepetition.qml	2016-03-07 17:57:04 +0000
> +++ EventRepetition.qml	2016-03-15 13:00:49 +0000
> @@ -19,14 +19,14 @@
>  import QtQuick 2.4
>  import QtOrganizer 5.0
>  import Ubuntu.Components 1.3
> -import Ubuntu.Components.ListItems 1.0 as ListItem
> +import Ubuntu.Components.ListItems 1.0 as ListItems

As Nik90 already mentioned on Telegram, we have to keep this until all ListItems are updated to 1.3

>  import Ubuntu.Components.Pickers 1.0
>  import QtOrganizer 5.0
>  import "Defines.js" as Defines
>  import "Recurrence.js" as Recurrence
>  
>  Page {
> -    id: repetition
> +    id: root
>  
>      property var weekDays : [];
>      property var eventRoot;
> @@ -92,67 +105,77 @@
>          recurrenceOption.selectedIndex = index;
>      }
>  
> -    head.backAction: Action{
> -        id:backAction
> -        iconName: "back"
> -        onTriggered: {
> -            var recurrenceRule = Defines.recurrenceValue[ recurrenceOption.selectedIndex ];
> -
> -            if (recurrenceRule !== RecurrenceRule.Invalid) {
> -                if (eventRoot.rule === null || eventRoot.rule === undefined ){
> -                    eventRoot.rule = Qt.createQmlObject("import QtOrganizer 5.0; RecurrenceRule {}", eventRoot.event.recurrence,"EventRepetition.qml");
> -                }
> -
> -                var rule = eventRoot.rule;
> -                rule.frequency = recurrenceRule;
> -                switch(recurrenceOption.selectedIndex){
> -                case 1: //daily
> -                case 2: //weekly
> -                case 3: //weekly
> -                case 4: //weekly
> -                case 5: //weekly
> -                    rule.daysOfWeek = eventUtils.getDaysOfWeek(recurrenceOption.selectedIndex, weekDays );
> -                    break;
> -                case 6: //monthly
> -                    rule.daysOfMonth = [eventRoot.date.getDate()];
> -                    break;
> -                case 7: //yearly
> -                    rule.monthsOfYear = [eventRoot.date.getMonth()];
> -                    rule.daysOfMonth = [eventRoot.date.getDate()];
> -                    break;
> -                case 0: //once
> -                default:
> -                    //it should not come here
> -                    break;
> -                }
> -
> -                if (limitOptions.selectedIndex === 1
> -                        && recurrenceOption.selectedIndex > 0
> -                        && limitCount.text != "") {
> -                    rule.limit =  parseInt(limitCount.text);
> -                }
> -                else if (limitOptions.selectedIndex === 2 && recurrenceOption.selectedIndex > 0) {
> -                    rule.limit =  datePick.date;
> -                }
> -                else {
> -                    rule.limit = undefined;
> -                }
> +    function onBackClicked() {
> +        var recurrenceRule = Defines.recurrenceValue[ recurrenceOption.selectedIndex ];
> +
> +        if (recurrenceRule !== RecurrenceRule.Invalid) {
> +            if (eventRoot.rule === null || eventRoot.rule === undefined ){
> +                eventRoot.rule = Qt.createQmlObject("import QtOrganizer 5.0; RecurrenceRule {}", eventRoot.event.recurrence,"EventRepetition.qml");
> +            }
> +
> +            var rule = eventRoot.rule;
> +            rule.frequency = recurrenceRule;
> +            switch(recurrenceOption.selectedIndex){
> +            case 1: //daily
> +            case 2: //weekly
> +            case 3: //weekly
> +            case 4: //weekly
> +            case 5: //weekly
> +                rule.daysOfWeek = eventUtils.getDaysOfWeek(recurrenceOption.selectedIndex, weekDays );
> +                break;
> +            case 6: //monthly
> +                rule.daysOfMonth = [eventRoot.date.getDate()];
> +                break;
> +            case 7: //yearly
> +                rule.monthsOfYear = [eventRoot.date.getMonth()];
> +                rule.daysOfMonth = [eventRoot.date.getDate()];
> +                break;
> +            case 0: //once
> +            default:
> +                //it should not come here
> +                break;
> +            }
> +
> +            if (limitOptions.selectedIndex === 1
> +                    && recurrenceOption.selectedIndex > 0
> +                    && limitCount.text != "") {
> +                rule.limit =  parseInt(limitCount.text);
> +            }
> +            else if (limitOptions.selectedIndex === 2 && recurrenceOption.selectedIndex > 0) {
> +                rule.limit =  datePick.date;
>              }
>              else {
> -                eventRoot.rule = null;
> +                rule.limit = undefined;
>              }
> -            pop()
> -        }
> +        }
> +        else {
> +            eventRoot.rule = null;
> +        }
> +        pop()
>      }
>  
> +
>      Column{
>          id:repeatColumn
>  
> -        anchors.fill: parent
> +        anchors {
> +            top: root.header.bottom
> +            bottom: root.bottom
> +            left: root.left
> +            right: root.right
> +        }
> +

I'll add Flickable to the Page so in case that column is higher then page user will be able to scroll the view.
Header will be fixed and flickable only enabled when content higher then page.height.

>          spacing: units.gu(1)
>  
> -        ListItem.Header{
> -            text: i18n.tr("Repeat")
> +        ListItem {
> +            height: repeatListItem.height
> +            divider.visible: false
> +
> +            ListItemLayout {
> +                id: repeatListItem
> +                title.text: i18n.tr("Repeat")
> +                title.color: Theme.palette.selected.overlayText
> +            }
>          }
>  
>          OptionSelector{


-- 
https://code.launchpad.net/~majster-pl/ubuntu-calendar-app/new-event-page/+merge/288637
Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app.


References