ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrea Bernabei <andrea.bernabei@xxxxxxxxxxxxx>]

minor comments from me, looks good otherwise, thanks :)

Diff comments:

> === modified file 'calendar.qml'
> --- calendar.qml	2015-01-22 20:07:30 +0000
> +++ calendar.qml	2015-05-28 12:57:38 +0000
> @@ -95,6 +95,21 @@
>      footerColor: "#ECECEC"
>      anchorToKeyboard: true
>  
> +    Connections {
> +        target: UriHandler
> +        onOpened: {
> +            var uri = uris[0]
> +            if(uri !== undefined && url != "") {
> +                var commands = uri.split("://")[1].split("=");
> +                if(commands[0].toLowerCase() === "eventid") {
> +                    // calendar://eventid=??
> +                    if( eventModel ) {
> +                        eventModel.showEventFromId(commands[1]);
> +                    }
> +                }
> +            }
> +        }
> +    }
>  
>      PageStack {
>          id: pageStack
> @@ -168,18 +183,29 @@
>                  collectionFilter.ids = collectionIds;
>              }
>  
> +            function showEventFromId(eventId) {
> +                if(eventId === undefined || eventId === "") {

code style: do you skip the whitespace after the "if" everywhere in this app? is it a personal preference? are core apps supposed to follow a particular code style convention, or just standard Qt ones? Just curious :)

> +                    return;
> +                }
> +
> +                var requestId = "";
> +                var callbackFunc = function(id,fetchedItems) {
> +                    if( requestId === id && fetchedItems.length > 0 ) {
> +                        var event = fetchedItems[0];

is this var creation needed?

> +                        pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event,"model": eventModel});
> +                    }
> +                    eventModel.onItemsFetched.disconnect( callbackFunc );
> +                }
> +
> +                eventModel.onItemsFetched.connect( callbackFunc );
> +                requestId = eventModel.fetchItems(eventId);
> +            }
> +
>              Component.onCompleted: {
>                  delayedApplyFilter();
>  
>                  if (args.values.eventid) {
> -                    var requestId = "";
> -                    eventModel.onItemsFetched.connect( function(id,fetchedItems) {
> -                        if( requestId === id && fetchedItems.length > 0 ) {
> -                            var event = fetchedItems[0];
> -                            pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event,"model": eventModel});
> -                        }
> -                    });
> -                    requestId = eventModel.fetchItems([args.values.eventid]);
> +                    showEventFromId(args.values.eventid);
>                  }
>              }
>          }
> 


-- 
https://code.launchpad.net/~pkunal-parmar/ubuntu-calendar-app/UrlInvocation/+merge/255234
Your team Ubuntu Calendar Developers is requested to review the proposed merge of lp:~pkunal-parmar/ubuntu-calendar-app/UrlInvocation into lp:ubuntu-calendar-app.