ubuntu-touch-coreapps-reviewers team mailing list archive

[Renato Araujo Oliveira Filho <renato.filho@xxxxxxxxxxxxx>]

some comments inline.

Diff comments:

> 
> === modified file 'EventDetails.qml'
> --- EventDetails.qml	2016-03-02 19:55:52 +0000
> +++ EventDetails.qml	2016-03-09 18:35:25 +0000
> @@ -32,45 +31,44 @@
>  
>      property var event
>      property var model
> +    property var collection: model.collection(event.collectionId);
>  
> -    anchors{
> -        left: parent.left
> -        right: parent.right
> -        bottom: parent.bottom
> +    header: PageHeader {
> +        title: i18n.tr("Event Details")
> +        flickable: flicable
> +        trailingActionBar.actions: Action {
> +            text: i18n.tr("Edit");
> +            objectName: "edit"
> +            iconName: "edit";
> +            enabled: !collection.extendedMetaData("collection-readonly")
> +            onTriggered: {
> +                if( event.itemType === Type.EventOccurrence ) {
> +                    var dialog = PopupUtils.open(Qt.resolvedUrl("EditEventConfirmationDialog.qml"),root,{"event": event});
> +                    dialog.editEvent.connect( function(eventId){
> +                        if( eventId === event.parentId ) {
> +                            showEditEventPage(internal.parentEvent, model)
> +                        } else {
> +                            showEditEventPage(event, model)
> +                        }
> +                    });
> +                } else {
> +                    showEditEventPage(event, model)
> +                }
> +            }
> +        }
>      }
>  
> -    flickable: null
> -
> -    title: i18n.tr("Event Details")
> -
>      Component.onCompleted: {
>          showEvent(event)
>      }
>  
> -    Connections{
> -        target: pageStack
> -        onCurrentPageChanged:{
> -            if( pageStack.currentPage === root) {
> -                showEvent(event)
> -            }
> -        }
> -    }
> -
> -    RemindersModel {
> -        id: reminderModel
> -    }
> -
> -    function updateCollection(event) {
> -
> -        var collection = model.collection( event.collectionId );
> -        calendarIndicator.color = collection.color
> -        eventInfo.color=collection.color
> -        // TRANSLATORS: the first parameter refers to the name of event calendar.
> -        calendarName.text = i18n.tr("%1 Calendar").arg( collection.name)
> -
> -        //disable edit in case of read only calendar
> -        if( collection.extendedMetaData("collection-readonly") === true ) {
> -            editAction.enabled = false
> +    Keys.onEscapePressed: {
> +        pageStack.pop();
> +    }
> +
> +    Keys.onPressed: {

You should use shortCut property on Edit action.

> +        if ((event.key === Qt.Key_E) && ( event.modifiers & Qt.ControlModifier)) {
> +            showEditEventPage(event, model);
>          }
>      }
>  
> @@ -223,37 +228,22 @@
>          })
>      }
>  
> -    Keys.onEscapePressed: {
> -        pageStack.pop();
> -    }
> -
> -    Keys.onPressed: {
> -        if ((event.key === Qt.Key_E) && ( event.modifiers & Qt.ControlModifier)) {
> -            showEditEventPage(event, model);
> -        }
> -    }
> -
> -    head.actions: [
> -        Action {
> -            text: i18n.tr("Edit");
> -            objectName: "edit"
> -            iconName: "edit";
> -            onTriggered: {
> -                if( event.itemType === Type.EventOccurrence ) {
> -                    var dialog = PopupUtils.open(Qt.resolvedUrl("EditEventConfirmationDialog.qml"),root,{"event": event});
> -                    dialog.editEvent.connect( function(eventId){
> -                        if( eventId === event.parentId ) {
> -                            showEditEventPage(internal.parentEvent, model)
> -                        } else {
> -                            showEditEventPage(event, model)
> -                        }
> -                    });
> -                } else {
> -                    showEditEventPage(event, model)
> -                }
> +    Connections{
> +        target: pageStack
> +        onCurrentPageChanged:{
> +            if( pageStack.currentPage === root) {

Could this be replaced by 'Page.onActiveChanged' ???
Could you add comments explaining why do you need this?

> +                showEvent(event)
>              }
>          }
> -    ]
> +    }
> +
> +    RemindersModel {
> +        id: reminderModel
> +    }
> +
> +    ListModel {
> +        id: contactModel
> +    }
>  
>      EventUtils{
>          id:eventUtils


-- 
https://code.launchpad.net/~nik90/ubuntu-calendar-app/revamp-eventdetails-page/+merge/288492
Your team Ubuntu Calendar Developers is requested to review the proposed merge of lp:~nik90/ubuntu-calendar-app/revamp-eventdetails-page into lp:ubuntu-calendar-app.