Peko Chen [:pekochen] Reporter
	Description • 11 years ago

Need to design a new button for today in order to seperate other tabs(month,week,day).

	Peko Chen [:pekochen] Reporter
	Comment 1 • 10 years ago

today btn spec updated.

	Peko Chen [:pekochen] Reporter
	Comment 2 • 10 years ago

images of today btn updated.

	Evan Tseng [:evanxd] Assignee
	Comment 4 • 10 years ago

Hi Harly,

It is the screenshot of visual refresh for today button.
And you could run on device with the https://github.com/mozilla-b2g/gaia/pull/18415 patch. Could you help me to review this.

Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 5 • 10 years ago

Hi Peko,

Just a little reminder for you.
We need the today button image with 1.5x and 2x size here.

Thanks.

	Miller Medeiros [:millermedeiros]
	Comment 6 • 10 years ago

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #5)
> Hi Peko,
> 
> Just a little reminder for you.
> We need the today button image with 1.5x and 2x size here.

Evan. We will use vectors for all icons going forward, see Bug 972871. so we need SVG. We should not merge this patch until we change it to use the custom font. I'll try to get the custom font merged as soon as possible so we can start adding more icons to it.

We will do most of the visual refresh on the "calendar-visual-refresh" branch since many changes causes side-effects (changing one view brakes another one) and we don't want people to report bugs before we complete the visual refresh. It's also a safe-guard in case some other team is not able to finish it before the release date (the plan is to do the visual refresh of all apps at once).

It is kinda hard to handle the visual refresh as separate pieces of work since the views are all related to each other and we kinda need to follow a "waterfall process" - too many cross dependencies...

Please always assign myself or gaye as reviewer for any patch related to the calendar app.

Thanks!

	Evan Tseng [:evanxd] Assignee
	Comment 7 • 10 years ago

Hi Miller,

For the vector icons, is the "Don't use SVG please" discussion https://groups.google.com/forum/#!topic/mozilla.dev.gaia/Qpg8tQBD_e0 not a issue for us?

Yes, many cross dependencies will be happened to do visual refresh. So we should follow the bug dependencies(https://bugzilla.mozilla.org/showdependencygraph.cgi?id=950209) to work on this.

For the reviewer things, could other Calendar module owner/peer not be supported to review code? Or you would like Gareth and you review all of visual refresh related bug to make sure the "waterfall process" work well?

	Evan Tseng [:evanxd] Assignee
	Comment 8 • 10 years ago

Thanks for the reminding of the vector icon things.

	Peko Chen [:pekochen] Reporter
	Comment 9 • 10 years ago

including 1x,1.5x,2x
thanks~

	Peko Chen [:pekochen] Reporter
	Comment 10 • 10 years ago

Hi Evan,

attached image is today btn (svg)
please have a look.
thank you~

	Evan Tseng [:evanxd] Assignee
	Comment 11 • 10 years ago

Hi Peko,
Thanks for the help. :)

	Evan Tseng [:evanxd] Assignee
	Comment 12 • 10 years ago

Comment on attachment 8408140 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla-b2g/gaia/pull/18415

Hi Miller,
Could you give me feedbacks for the patch?
Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 13 • 10 years ago

Comment on attachment 8408140 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla-b2g/gaia/pull/18415

Hi Miller,

Could you help me to review the patch.
Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 14 • 10 years ago

This bug depends on Bug 972876, because the today vector icon is in the patch for Bug 972876.

	Miller Medeiros [:millermedeiros]
	Comment 15 • 10 years ago

Hi Evan, please center the date and icon vertically inside the available area. (following the spec). Thanks.

	Miller Medeiros [:millermedeiros]
	Comment 16 • 10 years ago

Comment on attachment 8408140 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla-b2g/gaia/pull/18415

Evan, I added a few comments on github, please flag me again for review when you are done with the updates. Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 17 • 10 years ago

Comment on attachment 8408140 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla-b2g/gaia/pull/18415

Hi Miller,

Thanks for the review.
I already updated the patch for the comments.

But there is an issue happened here https://github.com/mozilla-b2g/gaia/pull/18415/files#diff-aebb1958df1b6b066d073763a38995e1R321. The date doesn't change once cross a new day. I will fix that in the patch. Could you give me feedbacks for the css part first? Thanks.

For the marionette tests, I think we should rebase the master branch to have the new view code in calendar-visual-refresh branch. Or we could do visual refresh without marionette test and fix the tests once we merge the visual refresh code in master branch? I refer to do rebase now, we should have working marionette test during doing visual refresh. How do you think?

	Miller Medeiros [:millermedeiros]
	Comment 18 • 10 years ago

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #17)
> Comment on attachment 8408140 [details] [review]
> Link to GitHub pull-request: https://github.com/mozilla-b2g/gaia/pull/18415

uhm. I thought I had rebased the calendar-visual-refresh last week, but it looks like the post-commit hook blocked the non-fast-forward push (or someone reverted my changes).. we really need to rebase this branch or start working out of master, we should write integration tests for the new features and changes whenever possible. As soon as they open the gaia tree for commits I'll try to force push the rebased branch or maybe we will need to delete the branch and create a new one.

the changes are looking way better, good work.

	Miller Medeiros [:millermedeiros]
	Comment 19 • 10 years ago

Hi Evan, talked with Gareth today and we decided to start landing the visual refresh patches to master branch in the next couple weeks. We created a thread on dev.gaia about it (https://groups.google.com/forum/#!topic/mozilla.dev.gaia/dmU739T86cU). I updated the custom font and I'm targeting the master branch on my new pull request (https://github.com/mozilla-b2g/gaia/pull/18611).

Rebase your changes and write some integration tests following the new structure for the today button (eg. check if date has the correct number)

Sorry for the extra work.

	Evan Tseng [:evanxd] Assignee
	Comment 20 • 10 years ago

Hi Miller,

Got you.
I will do that.

Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 22 • 10 years ago

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

Hi Miller,

Could you give me feedback for the cross new day issue?
I will add unit tests once make sure we are on the correct way.

	Miller Medeiros [:millermedeiros]
	Comment 23 • 10 years ago

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

added some comments to the PR, I think the "timer" logic (update today date after midnight) can be simplified/improved.

	Evan Tseng [:evanxd] Assignee
	Comment 24 • 10 years ago

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

Hi Miller,

Updated the patch for the comments.
Please help to review it.

Thanks.

	Miller Medeiros [:millermedeiros]
	Comment 25 • 10 years ago

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

added a few more notes, it's looking way better but I still think we can improve the tests. keep the good work.

	Evan Tseng [:evanxd] Assignee
	Comment 26 • 10 years ago

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

Hi Miller,

Updated the tests.
Please help to review the it.

Learned lots.
Thanks.

	Miller Medeiros [:millermedeiros]
	Comment 27 • 10 years ago

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

nice work!

	Evan Tseng [:evanxd] Assignee
	Comment 28 • 10 years ago

master: 27484401203780e782ced1064c351c2bf866d1db

	Evan Tseng [:evanxd] Assignee
	Comment 29 • 10 years ago

Hi Miller,
Thanks for the review.

	Evan Tseng [:evanxd] Assignee
	Comment 30 • 10 years ago

UI design is changed:
1. We would like to keep the color of tab items as #5F5F5F after users tap them.
2. The color of right border in today item is rgba(189,189,189, 0.2). It is the same as the color of tab's top border.

	Evan Tseng [:evanxd] Assignee
	Comment 31 • 10 years ago

Hi Harly,
Please help me to review this UI change.
Thanks.

	Harly Hsu[:harly]
	Comment 32 • 10 years ago

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

Great work, Evan!!

	Evan Tseng [:evanxd] Assignee
	Comment 33 • 10 years ago

Thanks for the review, Harly.

	Evan Tseng [:evanxd] Assignee
	Comment 34 • 10 years ago

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

Hi Miller,

UI design is changed as Comment 30.
So we have a patch for css changes.

Could you help me to review it?
Thanks.

	Evan Tseng [:evanxd] Assignee
	Comment 35 • 10 years ago

The UI changes are from Harly and Peko, and the patch is already ui-review+.

	Miller Medeiros [:millermedeiros]
	Comment 36 • 10 years ago

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

I would probably use a solid color (that matches the same tint) for the border (transparency is slower to paint), otherwise it looks good to me.

	Evan Tseng [:evanxd] Assignee
	Comment 37 • 10 years ago

Hi Miller,

Good point, learned.
Already changed for that.

Thanks for review.

	Evan Tseng [:evanxd] Assignee
	Comment 38 • 10 years ago

There are two patches for the bug.
master:
27484401203780e782ced1064c351c2bf866d1db
363c09a744281686a0c9bff9a80c53e06c7e11ba

	Edward Chen[:edchen]
	Comment 39 • 10 years ago

[Environment]
Gaia      f55fc5c507312c7aac51ec9bb73061fd4ed5c5fb
Gecko     https://hg.mozilla.org/mozilla-central/rev/3285e030d9c0
BuildID   20140504160202
Version   32.0a1
ro.build.version.incremental=324
ro.build.date=Thu Dec 19 14:04:55 CST 2013

[Result]
PASS