963394 - [Calendar] Event Detail_1.4 Visual Refresh

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect)

Comment 1

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_event_detail_spec.pdf

visual spec of event detail updated.

Comment 2

[Evan Tseng [:evanxd]]

Hi Miller,
I could help this.

Comment 3

[Evan Tseng [:evanxd]]

Hi Peko,
Could you provide the icons in event detail spec here?
Thanks.

Comment 4

[Peko Chen [:pekochen]]

Attachment: icon_event_details.zip

Hi Evan,

please see attachment for icons.
including arrow,download and attachment.
thank you~

Comment 5

[Peko Chen [:pekochen]]

Attachment: svg.zip

Hi Evan,

Here is svg format icon.
hope it works~~~
thank you.

Comment 6

[Evan Tseng [:evanxd]]

Hi Peko,
Nice, thanks. :)

Comment 7

[Evan Tseng [:evanxd]]

We could do the visual refresh without the invitation receipt feature(Bug 932254).

Comment 8

[Jason Smith [:jsmith]]

This shouldn't block - this is a feature, which is something we won't block on unless we're past FL & we're planning to still land the feature.

Comment 9

[Dylan Oliver [:doliver]]

Not a blocker but we are definitely working on this for 2.0 -- we are working on a better way to identify the features were are targeting for a particular release.

Evan, please continue to give this bug high priority for the current sprint.

Comment 10

[Evan Tseng [:evanxd]]

Hi Dylan,
Sure.

Comment 11

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_event_detail_spec.pdf

Hi Evan,

thanks for your reminding, please see attached file for the latest event detail spec.
the background color is white.
thank you so much.

Comment 12

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_event_detail_20140508_spec.pdf

Hi Even,

Harly and I made some changes, please see the latest spec for your reference.
thanks for your great help.

Peko

Comment 13

[Evan Tseng [:evanxd]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

WIP patch.

Comment 14

[Evan Tseng [:evanxd]]

Refer to Bug 940512 for calendar circle icon.

Comment 15

[Harly Hsu[:harly]]

Please modify the text "Remind me:" to "Reminder:" to avoid misunderstanding in different languages.

Comment 16

[Evan Tseng [:evanxd]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Hi Harly and Peko,

Could you help to review?

Thanks.

Comment 17

[Harly Hsu[:harly]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Nice work Evan, just need some minor changes to text within the sentence to lower case, and we are ready to go. Thanks

Comment 18

[Evan Tseng [:evanxd]]

https://github.com/evanxd/gaia/commit/02b1bd5a12a1d6c6b6c549b00c8eea2745fe75bf

Comment 19

[Miller Medeiros [:millermedeiros]]

Hi Evan, I added some comments on github related to the code (not a full code review tho, just things that I noticed quickly). I'm also adding the Day View visual refresh as a blocker since it will change the way the calendar colors are handled.

Comment 20

[Evan Tseng [:evanxd]]

Hi Miller,
Thanks for the comments.
I'm fix that.

Comment 21

[Evan Tseng [:evanxd]]

Hi Harly, Peko,

How do we handle the one and multiple alarms for the layout in the event detail view?

Comment 22

[Evan Tseng [:evanxd]]

After discussed with Harly and Peko in Person.
We have a new design for the alarm item.

Comment 23

[Evan Tseng [:evanxd]]

Attachment: one-alarm.png

Comment 24

[Evan Tseng [:evanxd]]

Attachment: multiple-alarm.png

Comment 25

[Evan Tseng [:evanxd]]

Hi Peko,

attachment 8420742 [details] and attachment 8420743 [details] is the screenshots for the alarm items.
Could you provide the spec here?

Thanks.

Comment 26

[Evan Tseng [:evanxd]]

There is an intermittent failure of marionette test for Contacts app. Refer to https://travis-ci.org/mozilla-b2g/gaia/jobs/24894974.

Comment 27

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_event_detail_20140512_spec.pdf

Hi Evan,

please check attached pdf for new design for the alarm item.
thanks for your help~

Comment 28

[Evan Tseng [:evanxd]]

Hi Peko,

Thanks for the spec.
Could you help to do the UI review?

Thanks.

Comment 29

[Peko Chen [:pekochen]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Well done :)
Thanks for your great help!!

Comment 30

[Evan Tseng [:evanxd]]

Thanks for the review. :)

Comment 31

[Evan Tseng [:evanxd]]

The commit for the UI review:
https://github.com/evanxd/gaia/commit/13cd1da0223dafc4c489456e0520ea236738c323

Comment 32

[Evan Tseng [:evanxd]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Hi Miller,

Could you help to review the patch?

Thanks.

Comment 33

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Hi Evan, I added a few comments to Github.

Comment 34

[Evan Tseng [:evanxd]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Hi Francesco,

Could you give feedback for the l10n part[1] of the patch?
Thanks.

[1] https://github.com/mozilla-b2g/gaia/pull/19058/files#diff-97b7fed57927556b91fb72d4d1b8c8b5R101

Comment 35

[Francesco Lodolo [:flod]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

Looks good to me, with only a request to add some comments about the strings.

Comment 36

[Evan Tseng [:evanxd]]

Hi Miller,

I updated the patch for the comments, and I added a new method `formatEndDate` in `apps/calendar/js/calc.js`.

[1] https://github.com/mozilla-b2g/gaia/pull/19058/files#diff-407b61bbaaed90fcd114d1e217dc21f7R71

Please help to review the patch, thanks.

Comment 37

[Evan Tseng [:evanxd]]

Hi Miller,

I updated the patch for the comments.
Could you review the patch again?

Thanks.

Comment 38

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8419293 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19058

one small comment on github and we should be good to go. good work Evan.

Comment 39

[Stephany Wilkes]

Nice work, everyone. Excited to see this land! :)

Comment 40

[Evan Tseng [:evanxd]]

Hi Miller,

I added comment[1] on GitHub for the multiple all day duration label.

[1] https://github.com/mozilla-b2g/gaia/pull/19058/files#diff-97b7fed57927556b91fb72d4d1b8c8b5R100

Comment 41

[Miller Medeiros [:millermedeiros]]

Evan, no need to change it. It's following the visual spec and makes total sense to keep it this way (so it always uses 2 lines for the event date). Sorry for the confusion. Thanks.

Comment 42

[Evan Tseng [:evanxd]]

Hi Miller,

No problem.
Thanks for the review.

Comment 43

[Evan Tseng [:evanxd]]

master: 793b7eaec42b0226395f87e87be14c6d50c36a49

Comment 44

[Gareth Aye [:gaye] (back from PTO)]

User facing changes need integration tests. There was an existing, disabled integration test which this change breaks as well. We should re-enable that integration test at the very least, and, if appropriate, add some additional coverage for the new read event view.

Comment 45

[Gareth Aye [:gaye] (back from PTO)]

:millermedeiros also suggested that we keep this in and re-enable and land changes to the relevant test code at a later point in time. This is an excerpt from an old discussion on dev-gaia regarding why it is most sensible to land changes to test code alongside changes to feature code https://gist.github.com/gaye/5f968e8bac21cc91f960.

Comment 46

[Gareth Aye [:gaye] (back from PTO)]

Reverted https://github.com/mozilla-b2g/gaia/commit/a921195442e783a9ccd41c605f1f6134e4b02ec1

Comment 47

[Miller Medeiros [:millermedeiros]]

Evan, I fixed and re-enabled the create_event_test (Bug 974731). This should at least make your job easier. Sorry for all the trouble.

Comment 48

[Evan Tseng [:evanxd]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20083

Hi Miller,

No problem.

Could you help me to review the patch.
The patch is based on the previous r+ patch, and I just add marionette tests.

Thanks.

Comment 49

[Miller Medeiros [:millermedeiros]]

Evan, FYI we added some comments on github.

Comment 50

[Evan Tseng [:evanxd]]

Hi Miller,

I updated the patch for the comments, and the travis job is here https://travis-ci.org/evanxd/gaia/builds/26919824.

I sent two pull requests to master branches of `mozilla-b2g` and `evanxd` with using same branch `evanxd:bug-963394`, and I think that is why the travis job run on `evanxd`'s travis not `mozilla-b2g`. It might be travis issues.

Comment 51

[Miller Medeiros [:millermedeiros]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20153/files

since we want to land this today and Evan is on a different timezone (Taipei) I updated the marionette tests and created a new PR.

Gareth, please review it as soon as possible and let me know what you think.

PS: kept Evan as the commit author since he did 99% of the work.

Comment 52

[Miller Medeiros [:millermedeiros]]

landed: https://github.com/mozilla-b2g/gaia/commit/b0aa62d9b8518e3dc8a14cd1e0b86f62470c87a9

Comment 53

[Evan Tseng [:evanxd]]

Thanks for Miller's and Gareth's help.

Comment 54

[Edward Chen[:edchen]]

[Environment]
Gaia      8d865839d932bfbd5e157f376f74d8cb12bfdd51
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/1d4046a8cb6c
BuildID   20140610000223
Version   32.0a2
ro.build.version.incremental=94
ro.build.date=Tue May 20 09:29:20 CST 2014

[Result]
Pass