Closed Bug 963394 Opened 6 years ago Closed 6 years ago

[Calendar] Event Detail_1.4 Visual Refresh

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.0, tracking-b2g:backlog)

VERIFIED FIXED
2.0 S3 (6june)
feature-b2g 2.0
tracking-b2g backlog

People

(Reporter: HHuang, Assigned: evanxd)

References

Details

(Whiteboard: [p=7], [priority])

Attachments

(7 files, 4 obsolete files)

No description provided.
Blocks: 950209
blocking-b2g: --- → backlog
visual spec of event detail updated.
Hi Miller,
I could help this.
Assignee: nobody → evanxd
Assignee: evanxd → nobody
Assignee: nobody → evanxd
Hi Peko,
Could you provide the icons in event detail spec here?
Thanks.
Flags: needinfo?(pchen)
Attached file icon_event_details.zip
Hi Evan,

please see attachment for icons.
including arrow,download and attachment.
thank you~
Flags: needinfo?(pchen)
Attached file svg.zip
Hi Evan,

Here is svg format icon.
hope it works~~~
thank you.
Hi Peko,
Nice, thanks. :)
We could do the visual refresh without the invitation receipt feature(Bug 932254).
Whiteboard: [p=5]
Target Milestone: --- → 2.0 S1 (9may)
blocking-b2g: backlog → 2.0+
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.
blocking-b2g: 2.0+ → 2.0?
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.
blocking-b2g: 2.0? → backlog
Whiteboard: [p=5] → [p=5], [priority]
Hi Dylan,
Sure.
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.
Attachment #8372159 - Attachment is obsolete: true
Hi Even,

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

Peko
Attachment #8419226 - Attachment is obsolete: true
Refer to Bug 940512 for calendar circle icon.
feature-b2g: --- → 2.0
Please modify the text "Remind me:" to "Reminder:" to avoid misunderstanding in different languages.
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.
Attachment #8419293 - Flags: ui-review?(hhsu)
Attachment #8419293 - Flags: ui-review?(pchen)
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
Attachment #8419293 - Flags: ui-review?(hhsu) → ui-review+
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.
Depends on: 951075
Hi Miller,
Thanks for the comments.
I'm fix that.
Hi Harly, Peko,

How do we handle the one and multiple alarms for the layout in the event detail view?
After discussed with Harly and Peko in Person.
We have a new design for the alarm item.
Attached image one-alarm.png
Attached image multiple-alarm.png
Hi Peko,

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

Thanks.
Flags: needinfo?(pchen)
There is an intermittent failure of marionette test for Contacts app. Refer to https://travis-ci.org/mozilla-b2g/gaia/jobs/24894974.
Hi Evan,

please check attached pdf for new design for the alarm item.
thanks for your help~
Flags: needinfo?(pchen)
Hi Peko,

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

Thanks.
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!!
Attachment #8419293 - Flags: ui-review?(pchen) → ui-review+
Thanks for the review. :)
Whiteboard: [p=5], [priority] → [p=7], [priority]
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
No longer depends on: 951075
Whiteboard: [p=7], [priority] → [p=5], [priority]
Target Milestone: 2.0 S2 (23may) → 2.0 S1 (9may)
Whiteboard: [p=5], [priority] → [p=7], [priority]
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
Status: NEW → ASSIGNED
Flags: in-moztrap+
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.
Attachment #8419293 - Flags: review?(mmedeiros)
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.
Flags: needinfo?(evanxd)
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
QA Contact: edchen
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
Attachment #8419293 - Flags: feedback?(francesco.lodolo)
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.
Attachment #8419293 - Flags: feedback?(francesco.lodolo) → feedback+
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.
Flags: needinfo?(evanxd)
Hi Miller,

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

Thanks.
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.
Attachment #8419293 - Flags: review?(mmedeiros) → review+
Nice work, everyone. Excited to see this land! :)
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
Flags: needinfo?(mmedeiros)
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.
Flags: needinfo?(mmedeiros)
Hi Miller,

No problem.
Thanks for the review.
master: 793b7eaec42b0226395f87e87be14c6d50c36a49
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
: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.
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.
Flags: needinfo?(evanxd)
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.
Attachment #8435012 - Flags: review?(mmedeiros)
Flags: needinfo?(evanxd)
Evan, FYI we added some comments on github.
Flags: needinfo?(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.
Flags: needinfo?(evanxd)
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.
Attachment #8419293 - Attachment is obsolete: true
Attachment #8435012 - Attachment is obsolete: true
Attachment #8435012 - Flags: review?(mmedeiros)
Attachment #8435910 - Flags: review?(gaye)
Attachment #8435910 - Flags: review?(gaye) → review+
landed: https://github.com/mozilla-b2g/gaia/commit/b0aa62d9b8518e3dc8a14cd1e0b86f62470c87a9
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Thanks for Miller's and Gareth's help.
[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
Status: RESOLVED → VERIFIED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.