Merge lp:~pkunal-parmar/ubuntu-calendar-app/weekview_ribbon into lp:ubuntu-calendar-app

Proposed by Kunal Parmar
Status: Merged
Approved by: Francis Ginther
Approved revision: 91
Merged at revision: 80
Proposed branch: lp:~pkunal-parmar/ubuntu-calendar-app/weekview_ribbon
Merge into: lp:ubuntu-calendar-app
Diff against target: 589 lines (+535/-1)
7 files modified
TimeLineBackground.qml (+43/-0)
TimeLineBase.qml (+99/-0)
TimeLineView.qml (+1/-1)
WeekComponent.qml (+131/-0)
WeekRibbon.qml (+133/-0)
WeekView.qml (+117/-0)
calendar.qml (+11/-0)
To merge this branch: bzr merge lp:~pkunal-parmar/ubuntu-calendar-app/weekview_ribbon
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Olivier Tilloy (community) Approve
Review via email: mp+174992@code.launchpad.net

Commit message

WeekRibbon Component created,
WeekView created

Currently I am pushing weekView as new page on to page stack. Later it will be changed to tab when new tab structure is implemented.

Description of the change

WeekRibbon Componenet created,
WeekView created

Currently I am pushing weekView as new page on to page stack. Later it will be changed to tab when new tab structure is implemented.

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Nice job! The new week view looks pretty good.
Just giving a quick test run, I’m seeing a couple of issues:

 1) file:///home/osomon/dev/phablet/core-apps/ubuntu-calendar-app/WeekView.qml:16: ReferenceError: event is not defined

 2) When switching to the week view, the first day of the week is selected by default, shouldn’t it be the current day instead?

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

544 + //mainFile: "WeekRibbon.qml"

please revert this useless change

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

When swiping left/right on the week ribbon, its position is animated, allowing to see what days the next/following week have while dragging.
However when swiping left/right on the lower part of the view, the week ribbon is not animated, its values change statically. Is that intended by design? It seems to me that it would look better if both elements were synchronized (but I’m not a designer, so my opinion is just that, an opinion).

Revision history for this message
Olivier Tilloy (osomon) wrote :

The same way I think the current day of the week should be selected by default when opening the week view, shouldn’t the view scroll to the current time of the day? It currently starts at 00:00. And I rarely have appointments at this time of the day :)

Revision history for this message
Olivier Tilloy (osomon) wrote :

26 + //text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH:mm"))

Please remove this commented out line, as it might be picked up by the translation system, assuming it is part of the comment above targeted at translators.

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

156 + //text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH:mm"))
157 + text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH"))

the same applies here

Revision history for this message
Olivier Tilloy (osomon) wrote :

> The same way I think the current day of the week should be selected by default
> when opening the week view, shouldn’t the view scroll to the current time of
> the day? It currently starts at 00:00. And I rarely have appointments at this
> time of the day :)

I see some code to scroll the view to 9 o’clock when the week changes, but that doesn’t work when initially opening the view.

Revision history for this message
Olivier Tilloy (osomon) wrote :

455 + Label{
456 + id: dummy;text: "SUN";visible: false;fontSize: "large"
457 + }

This doesn’t look right. It might be that in another locale/language, weekdays are abbreviated with more than 3 characters, or think about languages with an entirely different alphabet or representation system (e.g. Chinese).

I would say that the width of an item in the week ribbon needs to be the width of the ribbon itself divided by 7 (and this will scale better on other form-factors).

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> Nice job! The new week view looks pretty good.
> Just giving a quick test run, I’m seeing a couple of issues:
>
> 1) file:///home/osomon/dev/phablet/core-apps/ubuntu-calendar-
> app/WeekView.qml:16: ReferenceError: event is not defined
>

There was some unnecessary code, i removed it.
> 2) When switching to the week view, the first day of the week is selected by
> default, shouldn’t it be the current day instead?
I changed it to make current day the selected one else first day of week as current one. But actually in Weekview selected day does not signify anything, it has significant importance in DayView as you can change day by selecting it.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> The same way I think the current day of the week should be selected by default
> when opening the week view, shouldn’t the view scroll to the current time of
> the day? It currently starts at 00:00. And I rarely have appointments at this
> time of the day :)

This makes sense, now i changed scrolling to 9 o'clock or if week is current week then scroll to current time

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > The same way I think the current day of the week should be selected by
> default
> > when opening the week view, shouldn’t the view scroll to the current time
> of
> > the day? It currently starts at 00:00. And I rarely have appointments at
> this
> > time of the day :)
>
> I see some code to scroll the view to 9 o’clock when the week changes, but
> that doesn’t work when initially opening the view.

This is also addressed with scroll to current time changes

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> When swiping left/right on the week ribbon, its position is animated, allowing
> to see what days the next/following week have while dragging.
> However when swiping left/right on the lower part of the view, the week ribbon
> is not animated, its values change statically. Is that intended by design? It
> seems to me that it would look better if both elements were synchronized (but
> I’m not a designer, so my opinion is just that, an opinion).

This needs some changes in current design, lets treat this separately, I will also try to get some feedback from Lina, what she think of it.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> 455 + Label{
> 456 + id: dummy;text: "SUN";visible: false;fontSize: "large"
> 457 + }
>
> This doesn’t look right. It might be that in another locale/language, weekdays
> are abbreviated with more than 3 characters, or think about languages with an
> entirely different alphabet or representation system (e.g. Chinese).
>
> I would say that the width of an item in the week ribbon needs to be the width
> of the ribbon itself divided by 7 (and this will scale better on other form-
> factors).

I removed dependency on dummy label and now calculating width based on ribbon width only

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> FAILED: Continuous integration, rev:83
> http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/60/
> Executed test runs:
> UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests/120
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-precise-
> amd64-ci/9
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-quantal-
> amd64-ci/60
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-
> amd64-ci/60
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-amd64-ci/9
>
> Click here to trigger a rebuild:
> http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/60/rebuild

something is wrong with autopilot,

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/calendar_app/tests/test_calendar.py", line 23, in setUp
    self.ubuntusdk.get_qml_view().visible, Eventually(Equals(True)))
MismatchError: After 10.0 seconds test on QQuickView.visible failed: True != dbus.Boolean(False, variant_level=1)

When I ran locally first time it failed then it started working properly

Revision history for this message
Olivier Tilloy (osomon) wrote :

> 82. By Kunal Parmar 2 hours ago
> temp changes in newevent to enable save

Temporary changes for local testing are fine, but please make sure to revert them before pushing, those changes shouldn’t be part of this MR.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> something is wrong with autopilot,
>
> Traceback (most recent call last):
> File "/usr/lib/python2.7/dist-packages/calendar_app/tests/test_calendar.py",
> line 23, in setUp
> self.ubuntusdk.get_qml_view().visible, Eventually(Equals(True)))
> MismatchError: After 10.0 seconds test on QQuickView.visible failed: True !=
> dbus.Boolean(False, variant_level=1)
>
> When I ran locally first time it failed then it started working properly

This is part of the log:

  QXcbConnection: XCB error: 148 (Unknown), sequence: 148, resource id: 0, major code: 140 (Unknown), minor code: 20

Looks like some sort of X error in the test environment, I’d bet this isn’t reproducible, so re-running the tests should fix it. If you revert revision 82 and push, this will trigger a re-run of the CI job.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

Sorry about this failure, I was in the middle of a job update which has now been finished. I restarted the job.

> FAILED: Continuous integration, rev:84
> http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/63/
> Executed test runs:
> FAILURE: http://91.189.93.70:8080/job/generic-mediumtests/128/console
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-precise-
> amd64-ci/12
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-quantal-
> amd64-ci/63
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-
> amd64-ci/63
> SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-
> amd64-ci/12
>
> Click here to trigger a rebuild:
> http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/63/rebuild

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > something is wrong with autopilot,
> >
> > Traceback (most recent call last):
> > File "/usr/lib/python2.7/dist-
> packages/calendar_app/tests/test_calendar.py",
> > line 23, in setUp
> > self.ubuntusdk.get_qml_view().visible, Eventually(Equals(True)))
> > MismatchError: After 10.0 seconds test on QQuickView.visible failed: True !=
> > dbus.Boolean(False, variant_level=1)
> >
> > When I ran locally first time it failed then it started working properly
>
> This is part of the log:
>
> QXcbConnection: XCB error: 148 (Unknown), sequence: 148, resource id: 0,
> major code: 140 (Unknown), minor code: 20
>
> Looks like some sort of X error in the test environment, I’d bet this isn’t
> reproducible, so re-running the tests should fix it. If you revert revision 82
> and push, this will trigger a re-run of the CI job.

In latest build, I noticed following error

QXcbConnection: XCB error: 148 (Unknown), sequence: 148, resource id: 0, major code: 140 (Unknown), minor code: 20
+ killall qmlscene
+ autopilot run -o /tmp/test_results.xml -f xml -r -rd /tmp/ calendar_app
/tmp/hudson2311662262021543461.sh: line 59: autopilot: command not found

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> Sorry about this failure, I was in the middle of a job update which has now
> been finished. I restarted the job.
>
> > FAILED: Continuous integration, rev:84
> > http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/63/
> > Executed test runs:
> > FAILURE: http://91.189.93.70:8080/job/generic-mediumtests/128/console
> > SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-precise-
> > amd64-ci/12
> > SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-quantal-
> > amd64-ci/63
> > SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-
> > amd64-ci/63
> > SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-
> > amd64-ci/12
> >
> > Click here to trigger a rebuild:
> > http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/63/rebuild

ahh, Thanks :)

Revision history for this message
Olivier Tilloy (osomon) wrote :

So when opening the week view the first time, the current day is not selected, and the horizontal orange line that indicates the current time is not visible. If I swipe to the left to go to next week and then swipe to the right to go back to the current week, both issues are fixed, so I guess it’s just a matter of calling whatever function does the scrolling and selection when the view is created.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> So when opening the week view the first time, the current day is not selected,
> and the horizontal orange line that indicates the current time is not visible.
> If I swipe to the left to go to next week and then swipe to the right to go
> back to the current week, both issues are fixed, so I guess it’s just a matter
> of calling whatever function does the scrolling and selection when the view is
> created.

Strange, for me only indicator for current time is not coming first time, but current day is selected.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > So when opening the week view the first time, the current day is not
> selected,
> > and the horizontal orange line that indicates the current time is not
> visible.
> > If I swipe to the left to go to next week and then swipe to the right to go
> > back to the current week, both issues are fixed, so I guess it’s just a
> matter
> > of calling whatever function does the scrolling and selection when the view
> is
> > created.
>
> Strange, for me only indicator for current time is not coming first time, but
> current day is selected.

And time indicator is not coming because, on creation time its not getting proper parent's width thus calculation is getting disrupted.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > > So when opening the week view the first time, the current day is not
> > selected,
> > > and the horizontal orange line that indicates the current time is not
> > visible.
> > > If I swipe to the left to go to next week and then swipe to the right to
> go
> > > back to the current week, both issues are fixed, so I guess it’s just a
> > matter
> > > of calling whatever function does the scrolling and selection when the
> view
> > is
> > > created.
> >
> > Strange, for me only indicator for current time is not coming first time,
> but
> > current day is selected.
>
> And time indicator is not coming because, on creation time its not getting
> proper parent's width thus calculation is getting disrupted.

Its width doesn’t have to be calculated dynamically, does it? Only its vertical position has to be set in a functional way.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

146 + function createSeparator(hour) {

Can you please rename this function? It’s not actually creating a separator, it’s only showing it.

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> 146 + function createSeparator(hour) {
>
> Can you please rename this function? It’s not actually creating a separator,
> it’s only showing it.

ok, I changed it to, showSeparator()

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > So when opening the week view the first time, the current day is not
> selected,
> > and the horizontal orange line that indicates the current time is not
> visible.
> > If I swipe to the left to go to next week and then swipe to the right to go
> > back to the current week, both issues are fixed, so I guess it’s just a
> matter
> > of calling whatever function does the scrolling and selection when the view
> is
> > created.
>
> Strange, for me only indicator for current time is not coming first time, but
> current day is selected.

Not seeing the current day initially selected (by default when first opening the week view the first day of the week, Monday here, is selected). If I swipe to next week and then back to the current week, the current day is correctly selected. So the issue is with the initial selection when the component is created.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > > So when opening the week view the first time, the current day is not
> > selected,
> > > and the horizontal orange line that indicates the current time is not
> > visible.
> > > If I swipe to the left to go to next week and then swipe to the right to
> go
> > > back to the current week, both issues are fixed, so I guess it’s just a
> > matter
> > > of calling whatever function does the scrolling and selection when the
> view
> > is
> > > created.
> >
> > Strange, for me only indicator for current time is not coming first time,
> but
> > current day is selected.
>
> Not seeing the current day initially selected (by default when first opening
> the week view the first day of the week, Monday here, is selected). If I swipe
> to next week and then back to the current week, the current day is correctly
> selected. So the issue is with the initial selection when the component is
> created.

I already modified code to handle the current day selection and for me it works fine.

http://www.youtube.com/watch?v=7R0TwkaS4lU

Do you think, I am missing something or its related to some other problem.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > > > So when opening the week view the first time, the current day is not
> > > selected,
> > > > and the horizontal orange line that indicates the current time is not
> > > visible.
> > > > If I swipe to the left to go to next week and then swipe to the right to
> > go
> > > > back to the current week, both issues are fixed, so I guess it’s just a
> > > matter
> > > > of calling whatever function does the scrolling and selection when the
> > view
> > > is
> > > > created.
> > >
> > > Strange, for me only indicator for current time is not coming first time,
> > but
> > > current day is selected.
> >
> > Not seeing the current day initially selected (by default when first opening
> > the week view the first day of the week, Monday here, is selected). If I
> swipe
> > to next week and then back to the current week, the current day is correctly
> > selected. So the issue is with the initial selection when the component is
> > created.
>
> I already modified code to handle the current day selection and for me it
> works fine.
>
> http://www.youtube.com/watch?v=7R0TwkaS4lU
>
> Do you think, I am missing something or its related to some other problem.

I don’t know what’s going on, but I confirm I can reproduce the issue reliably 100% of the times.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > > > > So when opening the week view the first time, the current day is not
> > > > selected,
> > > > > and the horizontal orange line that indicates the current time is not
> > > > visible.
> > > > > If I swipe to the left to go to next week and then swipe to the right
> to
> > > go
> > > > > back to the current week, both issues are fixed, so I guess it’s just
> a
> > > > matter
> > > > > of calling whatever function does the scrolling and selection when the
> > > view
> > > > is
> > > > > created.
> > > >
> > > > Strange, for me only indicator for current time is not coming first
> time,
> > > but
> > > > current day is selected.
> > >
> > > Not seeing the current day initially selected (by default when first
> opening
> > > the week view the first day of the week, Monday here, is selected). If I
> > swipe
> > > to next week and then back to the current week, the current day is
> correctly
> > > selected. So the issue is with the initial selection when the component is
> > > created.
> >
> > I already modified code to handle the current day selection and for me it
> > works fine.
> >
> > http://www.youtube.com/watch?v=7R0TwkaS4lU
> >
> > Do you think, I am missing something or its related to some other problem.
>
> I don’t know what’s going on, but I confirm I can reproduce the issue reliably
> 100% of the times.

ok, then in this case, I can disable highlight, as such highlight does not signify anything right now. It does not indicate anything when we change week nor does it do anything.
or we need to find out what is causing this behavior, i will try to add some logs in this case and then we can debug it further.

Revision history for this message
Olivier Tilloy (osomon) wrote :

In WeekComponent.qml, you’re referring to an 'intern' object, which is not defined there, but apparently in WeekView.qml, which is the parent item for WeekComponent.
This breaks encapsulation, as WeekComponent doesn’t know of WeekView.
In this case, considering that intern is a pretty generic object that only contains information about the current date, I guess there is no need for it to be shared as passed around as a property, so you should probably just duplicate it in WeekComponent.

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

It looks like function setSelectedDay() in WeekComponent.qml is never called anywhere. Can it be removed?

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > I don’t know what’s going on, but I confirm I can reproduce the issue
> reliably
> > 100% of the times.
>
> ok, then in this case, I can disable highlight, as such highlight does not
> signify anything right now. It does not indicate anything when we change week
> nor does it do anything.
> or we need to find out what is causing this behavior, i will try to add some
> logs in this case and then we can debug it further.

Looking at the implementation of WeekRibbon.qml, setSelectedDay() is called whenever visibleWeek is changed. I’d bet the initial value assignment doesn’t emit a Changed() signal, and thus setSelectedDay() never gets called when initially instantiating the view. How about adding a call to setSelectedDay in Component.onCompleted()?

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> It looks like function setSelectedDay() in WeekComponent.qml is never called
> anywhere. Can it be removed?

I think, This will be needed with DayView, when day is changed this function is supposed to be called. But in current implementation is not need.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > It looks like function setSelectedDay() in WeekComponent.qml is never called
> > anywhere. Can it be removed?
>
> I think, This will be needed with DayView, when day is changed this function
> is supposed to be called. But in current implementation is not need.

Ahh, I did not read that you were talking about WeekComponent.qml,

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > > It looks like function setSelectedDay() in WeekComponent.qml is never
> called
> > > anywhere. Can it be removed?
> >
> > I think, This will be needed with DayView, when day is changed this function
> > is supposed to be called. But in current implementation is not need.
>
> Ahh, I did not read that you were talking about WeekComponent.qml,

Looks like I was confused about WeekRibbon and WeekComponent and copied WeekRibbon's code to WeekComponent. I removed that function

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> In WeekComponent.qml, you’re referring to an 'intern' object, which is not
> defined there, but apparently in WeekView.qml, which is the parent item for
> WeekComponent.
> This breaks encapsulation, as WeekComponent doesn’t know of WeekView.
> In this case, considering that intern is a pretty generic object that only
> contains information about the current date, I guess there is no need for it
> to be shared as passed around as a property, so you should probably just
> duplicate it in WeekComponent.

This also looks like related to setSelectedDay in WeekComponent ? I removed that function, so this should also be addressed

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> > > I don’t know what’s going on, but I confirm I can reproduce the issue
> > reliably
> > > 100% of the times.
> >
> > ok, then in this case, I can disable highlight, as such highlight does not
> > signify anything right now. It does not indicate anything when we change
> week
> > nor does it do anything.
> > or we need to find out what is causing this behavior, i will try to add some
> > logs in this case and then we can debug it further.
>
> Looking at the implementation of WeekRibbon.qml, setSelectedDay() is called
> whenever visibleWeek is changed. I’d bet the initial value assignment doesn’t
> emit a Changed() signal, and thus setSelectedDay() never gets called when
> initially instantiating the view. How about adding a call to setSelectedDay in
> Component.onCompleted()?

but selectedDay is already assigned now as initial value, I think it need not be reassigned at Component.onCompleted
...
property var selectedDay: now;

However, I added Componet.onCompleted and calling setSelectedDay method from there. I also added logs, if issue happens kindy provide log, else I will remove those logs.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

The call to setSelectedDay() in Component.onCompleted seems to have done the trick, as it’s now working for me.
You can remove the logs.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > In WeekComponent.qml, you’re referring to an 'intern' object, which is not
> > defined there, but apparently in WeekView.qml, which is the parent item for
> > WeekComponent.
> > This breaks encapsulation, as WeekComponent doesn’t know of WeekView.
> > In this case, considering that intern is a pretty generic object that only
> > contains information about the current date, I guess there is no need for it
> > to be shared as passed around as a property, so you should probably just
> > duplicate it in WeekComponent.
>
> This also looks like related to setSelectedDay in WeekComponent ? I removed
> that function, so this should also be addressed

Indeed. Thanks.

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> The call to setSelectedDay() in Component.onCompleted seems to have done the
> trick, as it’s now working for me.
> You can remove the logs.

I removed the logs, thanks

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Looks good now. Thanks!

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

Just fixed the out of space error. Should be good to go.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

Had an incomplete fix, it should work now.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'TimeLineBackground.qml'
2--- TimeLineBackground.qml 1970-01-01 00:00:00 +0000
3+++ TimeLineBackground.qml 2013-08-01 12:41:40 +0000
4@@ -0,0 +1,43 @@
5+import QtQuick 2.0
6+import Ubuntu.Components 0.1
7+
8+Column {
9+ width: parent.width
10+ Repeater{
11+ model: 24 // hour in a day
12+
13+ delegate: Item {
14+ id: delegate
15+ width: parent.width
16+ // FIXME: get hour hight from somewhere
17+ height: units.gu(10)
18+
19+ Row {
20+ width: parent.width
21+ y: -timeLabel.height/2
22+ Label{
23+ id: timeLabel
24+ // TRANSLATORS: this is a time formatting string,
25+ // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions
26+ text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH"))
27+ color:"gray"
28+ anchors.top: parent.top
29+ }
30+ Rectangle{
31+ width: parent.width - timeLabel.width
32+ height:units.dp(1)
33+ color:"gray"
34+ anchors.verticalCenter: parent.verticalCenter
35+ }
36+ }
37+
38+ Rectangle{
39+ width: parent.width - units.gu(5)
40+ height:units.dp(1)
41+ color:"gray"
42+ anchors.verticalCenter: parent.verticalCenter
43+ anchors.horizontalCenter: parent.horizontalCenter
44+ }
45+ }
46+ }
47+}
48
49=== added file 'TimeLineBase.qml'
50--- TimeLineBase.qml 1970-01-01 00:00:00 +0000
51+++ TimeLineBase.qml 2013-08-01 12:41:40 +0000
52@@ -0,0 +1,99 @@
53+import QtQuick 2.0
54+import Ubuntu.Components 0.1
55+import "dateExt.js" as DateExt
56+
57+Item {
58+ id: bubbleOverLay
59+
60+ property var delegate;
61+ property var day;
62+ property int hourHeight: units.gu(10)
63+
64+ EventListModel {
65+ id: model
66+ termStart: bubbleOverLay.day
67+ termLength: Date.msPerDay
68+
69+ onReload: {
70+ bubbleOverLay.createEvents();
71+ }
72+ }
73+
74+ TimeSeparator{
75+ id: separator
76+ objectName: "separator"
77+ width: bubbleOverLay.width
78+ }
79+
80+ QtObject {
81+ id: intern
82+ property var eventMap;
83+ property var now : new Date();
84+ }
85+
86+ function showEventDetails(hour) {
87+ var event = intern.eventMap[hour];
88+ pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event});
89+ }
90+
91+ function createEventMap() {
92+ var eventMap = {};
93+ for(var i = 0 ; i < model.count ; ++i) {
94+ var event = model.get(i);
95+ eventMap[event.startTime.getHours()] = event
96+ }
97+ return eventMap;
98+ }
99+
100+ function createEvents() {
101+ intern.eventMap = createEventMap();
102+
103+ bubbleOverLay.destroyAllChildren();
104+
105+ for( var i=0; i < 24; ++i ) {
106+ var event = intern.eventMap[i];
107+ if( event ) {
108+ bubbleOverLay.createEvent(event,i);
109+ }
110+
111+ if( i === intern.now.getHours()
112+ && intern.now.isSameDay( bubbleOverLay.day )) {
113+ bubbleOverLay.showSeparator(i);
114+ }
115+ }
116+ }
117+
118+ function destroyAllChildren() {
119+ for( var i = children.length - 1; i >= 0; --i ) {
120+ if( children[i].objectName === "separator") {
121+ children[i].visible = false;
122+ } else {
123+ children[i].destroy();
124+ }
125+ }
126+ }
127+
128+ function createEvent( event ,hour) {
129+ var eventBubble = delegate.createObject(bubbleOverLay);
130+
131+ eventBubble.clicked.connect( bubbleOverLay.showEventDetails );
132+
133+ eventBubble.title = event.title;
134+ eventBubble.location = "Test"//event.location;
135+ eventBubble.hour = hour;
136+
137+ var yPos = (( event.startTime.getMinutes() * hourHeight) / 60) + hour * hourHeight
138+ eventBubble.y = yPos;
139+
140+ var durationMin = (event.endTime.getHours() - event.startTime.getHours()) * 60;
141+ durationMin += (event.endTime.getMinutes() - event.startTime.getMinutes());
142+ var height = (durationMin * hourHeight )/ 60;
143+ eventBubble.height = height;
144+ }
145+
146+ function showSeparator(hour) {
147+ var y = ((intern.now.getMinutes() * hourHeight) / 60) + hour * hourHeight;
148+ separator.y = y;
149+ separator.visible = true;
150+ }
151+}
152
153=== modified file 'TimeLineView.qml'
154--- TimeLineView.qml 2013-06-02 02:33:33 +0000
155+++ TimeLineView.qml 2013-08-01 12:41:40 +0000
156@@ -100,7 +100,7 @@
157 id: timeLabel
158 // TRANSLATORS: this is a time formatting string,
159 // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions
160- text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH:mm"))
161+ text: new Date(0, 0, 0, index).toLocaleTimeString(Qt.locale(), i18n.tr("HH"))
162 color:"gray"
163 anchors.top: parent.top
164 }
165
166=== added file 'WeekComponent.qml'
167--- WeekComponent.qml 1970-01-01 00:00:00 +0000
168+++ WeekComponent.qml 2013-08-01 12:41:40 +0000
169@@ -0,0 +1,131 @@
170+import QtQuick 2.0
171+import Ubuntu.Components 0.1
172+
173+import "dateExt.js" as DateExt
174+import "dataService.js" as DataService
175+
176+Flickable{
177+ id: timeLineView
178+
179+ property var weekStart: new Date()
180+ property int weekWidth:0;
181+
182+ contentHeight: timeLineColumn.height + units.gu(3)
183+ contentWidth: width
184+
185+ clip: true
186+
187+ onWeekStartChanged: {
188+ scroll();
189+ }
190+
191+ function scroll() {
192+ //scroll to 9 o'clock or to now
193+ var now = new Date();
194+ var hour = 9
195+ if( weekStart !== undefined
196+ && now.weekStart(Qt.locale().firstDayOfWeek).isSameDay(weekStart)) {
197+ hour = now.getHours();
198+ }
199+
200+ timeLineView.contentY = hour * units.gu(10);
201+ if(timeLineView.contentY >= timeLineView.contentHeight - timeLineView.height) {
202+ timeLineView.contentY = timeLineView.contentHeight - timeLineView.height
203+ }
204+ }
205+
206+ //scroll in case content height changed
207+ onContentHeightChanged: {
208+ scroll()
209+ }
210+
211+ Rectangle{
212+ id: background;
213+ anchors.fill: parent
214+ color: "white"
215+ }
216+
217+ TimeLineBackground{
218+ id: timeLineColumn
219+ anchors.top: parent.top
220+ anchors.topMargin: units.gu(3)
221+ width: parent.width
222+ }
223+
224+ //vertical lines for weeks
225+ Row{
226+ id: dayIndicator
227+
228+ x: timeLabel.width
229+ width: parent.width
230+ height: timeLineView.contentHeight
231+
232+ Repeater{
233+ model:7
234+ delegate: Rectangle{
235+ height: dayIndicator.height
236+ width: weekWidth
237+ border.color: "gray"
238+ opacity: 0.1
239+ }
240+ }
241+ }
242+
243+ Row{
244+ id: week
245+ width: timeLineColumn.width - x
246+ height: timeLineColumn.height
247+ anchors.top: parent.top
248+ anchors.topMargin: units.gu(3)
249+ x: timeLabel.width
250+ spacing: 0
251+
252+ property var weekStartDay: timeLineView.weekStart.weekStart( Qt.locale().firstDayOfWeek );
253+
254+ Repeater{
255+ model: 7
256+
257+ delegate: TimeLineBase {
258+ anchors.top: parent.top
259+ height: parent.height
260+ width: weekWidth
261+ delegate: infoBubbleComponent
262+ day: week.weekStartDay.addDays(index)
263+ }
264+ }
265+ }
266+
267+ Component{
268+ id: infoBubbleComponent
269+ Rectangle{
270+ id: infoBubble
271+
272+ property string title;
273+ property string location;
274+ property int hour;
275+
276+ signal clicked(int hour);
277+
278+ color:'#fffdaa';
279+ width: weekWidth
280+ x: units.gu(0)
281+
282+ border.color: "#f4d690"
283+
284+ Label{
285+ text:infoBubble.title;
286+ fontSize:"small";
287+ color:"black"
288+ wrapMode: Text.WrapAtWordBoundaryOrAnywhere
289+ width: parent.width
290+ }
291+
292+ MouseArea{
293+ anchors.fill: parent
294+ onClicked: {
295+ infoBubble.clicked(hour);
296+ }
297+ }
298+ }
299+ }
300+}
301
302=== added file 'WeekRibbon.qml'
303--- WeekRibbon.qml 1970-01-01 00:00:00 +0000
304+++ WeekRibbon.qml 2013-08-01 12:41:40 +0000
305@@ -0,0 +1,133 @@
306+import QtQuick 2.0
307+import Ubuntu.Components 0.1
308+
309+import "dateExt.js" as DateExt
310+import "colorUtils.js" as Color
311+
312+PathViewBase{
313+ id: weekRibbonRoot
314+
315+ property int weekWidth:0;
316+ property var visibleWeek: intern.now
317+
318+ signal daySelected(var day);
319+ signal weekChanged(var visibleWeek);
320+
321+ QtObject{
322+ id: intern
323+ property var now: new Date()
324+ property int weekstartDay: Qt.locale().firstDayOfWeek
325+ property var weekStart: visibleWeek.addDays(-7)
326+ property var selectedDay: now;
327+ }
328+
329+ onNextItemHighlighted: {
330+ nextWeek();
331+ }
332+
333+ onPreviousItemHighlighted: {
334+ previousWeek();
335+ }
336+
337+ onVisibleWeekChanged: {
338+ setSelectedDay();
339+ }
340+
341+ Component.onCompleted: {
342+ setSelectedDay();
343+ }
344+
345+ function nextWeek() {
346+ var weekStartDay= visibleWeek.weekStart(intern.weekstartDay);
347+ visibleWeek = weekStartDay.addDays(7);
348+ setSelectedDay();
349+
350+ weekChanged( visibleWeek );
351+ }
352+
353+ function previousWeek(){
354+ var weekStartDay = visibleWeek.weekStart(intern.weekstartDay);
355+ visibleWeek = weekStartDay.addDays(-7);
356+ setSelectedDay();
357+
358+ weekChanged( visibleWeek );
359+ }
360+
361+ function setSelectedDay() {
362+ if( intern.now.weekStart( intern.weekstartDay).isSameDay(visibleWeek) ) {
363+ intern.selectedDay = intern.now
364+ } else {
365+ intern.selectedDay = visibleWeek
366+ }
367+ }
368+
369+ delegate: Row{
370+ id: dayLabelRow
371+ width: parent.width
372+
373+ function getWeekStart() {
374+ if (index === weekRibbonRoot.currentIndex) {
375+ return intern.weekStart;
376+ }
377+ var previousIndex = weekRibbonRoot.currentIndex > 0 ? weekRibbonRoot.currentIndex - 1 : 2
378+
379+ if ( index === previousIndex ) {
380+ var weekStartDay= intern.weekStart.weekStart( Qt.locale().firstDayOfWeek);
381+ return weekStartDay.addDays(-7);
382+ }
383+
384+ var weekStartDay = intern.weekStart.weekStart( Qt.locale().firstDayOfWeek);
385+ return weekStartDay.addDays(7);
386+ }
387+
388+ property var weekStart: getWeekStart();
389+
390+ Repeater{
391+ id: dayLabelRepeater
392+ model:7
393+ delegate: dafaultDayLabelComponent
394+ }
395+ }
396+
397+ Component{
398+ id: dafaultDayLabelComponent
399+
400+ Rectangle{
401+ id: weekDay
402+
403+ width: column.width
404+ height: column.height
405+
406+ color: intern.selectedDay.isSameDay(day) ? Color.ubuntuOrange : "white"
407+
408+ property var weekStartDay: parent.weekStart.weekStart( Qt.locale().firstDayOfWeek);
409+ property var day : weekStartDay.addDays(index)
410+
411+ Column {
412+ id: column
413+ width: weekWidth
414+ Label{
415+ text: Qt.locale().standaloneDayName(( intern.weekstartDay + index), Locale.ShortFormat)
416+ horizontalAlignment: Text.AlignHCenter
417+ width: column.width
418+ fontSize: "medium"
419+ }
420+ Label{
421+ text: weekDay.day.getDate()
422+ horizontalAlignment: Text.AlignHCenter
423+ width: column.width
424+ fontSize: "large"
425+ }
426+ }
427+
428+ MouseArea{
429+ anchors.fill: parent
430+
431+ onClicked: {
432+ intern.selectedDay = day
433+ weekRibbonRoot.daySelected(day);
434+ }
435+ }
436+ }
437+ }
438+}
439
440=== added file 'WeekView.qml'
441--- WeekView.qml 1970-01-01 00:00:00 +0000
442+++ WeekView.qml 2013-08-01 12:41:40 +0000
443@@ -0,0 +1,117 @@
444+import QtQuick 2.0
445+import Ubuntu.Components 0.1
446+
447+import "dateExt.js" as DateExt
448+import "dataService.js" as DataService
449+
450+Page{
451+ id: root
452+ anchors.fill: parent
453+
454+ property var dayStart: new Date()
455+
456+ Component.onCompleted: {
457+ if( pageStack.header )
458+ pageStack.header.visible = false;
459+ }
460+
461+ Component.onDestruction: {
462+ if( pageStack.header )
463+ pageStack.header.visible = true;
464+ }
465+
466+ onDayStartChanged:{
467+ weekRibbon.visibleWeek = dayStart.weekStart(intern.firstDayOfWeek);
468+ weekViewPath.visibleWeek = dayStart.weekStart(intern.firstDayOfWeek);
469+ }
470+
471+ Label{
472+ id: todayLabel
473+ text: Qt.formatDateTime( new Date(),"d MMMM yyyy");
474+ fontSize: "large"
475+ width: parent.width
476+ }
477+
478+ Label{
479+ id: timeLabel;visible: false
480+ text: new Date(0, 0, 0, 0).toLocaleTimeString(Qt.locale(), i18n.tr("HH"))
481+ }
482+
483+ WeekRibbon{
484+ id: weekRibbon
485+ visibleWeek: dayStart.weekStart(intern.firstDayOfWeek);
486+ anchors.top: todayLabel.bottom
487+ anchors.left: timeLabel.right
488+ width: parent.width
489+ height: units.gu(10)
490+ //removing timeLabel.width from front and back of ribbon
491+ weekWidth: ((width - 2* timeLabel.width )/ 7 )
492+
493+ onWeekChanged: {
494+ dayStart = visibleWeek
495+ }
496+ }
497+
498+ PathViewBase{
499+ id: weekViewPath
500+
501+ property var visibleWeek: dayStart.weekStart(intern.firstDayOfWeek);
502+
503+ QtObject{
504+ id: intern
505+ property int firstDayOfWeek: Qt.locale().firstDayOfWeek
506+ property var weekStart: weekViewPath.visibleWeek.addDays(-7)
507+ }
508+
509+ anchors.top: weekRibbon.bottom
510+ width: parent.width
511+ height: parent.height - weekRibbon.height - units.gu(3)
512+
513+ onNextItemHighlighted: {
514+ nextWeek();
515+ }
516+
517+ onPreviousItemHighlighted: {
518+ previousWeek();
519+ }
520+
521+ function nextWeek() {
522+ var weekStartDay = visibleWeek.weekStart(intern.firstDayOfWeek);
523+ visibleWeek = weekStartDay.addDays(7);
524+
525+ dayStart = visibleWeek
526+ }
527+
528+ function previousWeek(){
529+ var weekStartDay = visibleWeek.weekStart(intern.firstDayOfWeek);
530+ visibleWeek = weekStartDay.addDays(-7);
531+
532+ dayStart = visibleWeek
533+ }
534+
535+ delegate: WeekComponent {
536+ id: timeLineView
537+
538+ width: parent.width
539+ height: parent.height
540+ weekWidth: weekRibbon.weekWidth //dummy.width + units.gu(1)
541+ weekStart: getWeekStart();
542+
543+ function getWeekStart() {
544+ if (index === weekViewPath.currentIndex) {
545+ return intern.weekStart;
546+ }
547+ var previousIndex = weekViewPath.currentIndex > 0 ? weekViewPath.currentIndex - 1 : 2
548+
549+ if ( index === previousIndex ) {
550+ var weekStartDay= intern.weekStart.weekStart(intern.firstDayOfWeek);
551+ return weekStartDay.addDays(14);
552+ }
553+
554+ var weekStartDay = intern.weekStart.weekStart(intern.firstDayOfWeek);
555+ return weekStartDay.addDays(7);
556+ }
557+ }
558+ }
559+}
560+
561
562=== modified file 'calendar.qml'
563--- calendar.qml 2013-07-17 12:46:41 +0000
564+++ calendar.qml 2013-08-01 12:41:40 +0000
565@@ -63,6 +63,11 @@
566 text: i18n.tr("Year view")
567 onTriggered: pageStack.push(yearView);
568 }
569+ Action {
570+ iconSource: Qt.resolvedUrl("avatar.png")
571+ text: i18n.tr("Week view")
572+ onTriggered: pageStack.push(weekView);
573+ }
574 }
575
576 Tabs {
577@@ -134,6 +139,12 @@
578 }
579 }
580 }
581+
582+ Component{
583+ id: weekView
584+ WeekView{
585+ }
586+ }
587 }
588 }
589 }

Subscribers

People subscribed via source and target branches

to status/vote changes: