1007546 - [Calendar] Add/Edit Event 2.0 Visual Refresh

Status: VERIFIED FIXED

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

Comment 1

[Evan Tseng [:evanxd]]

Hi Peko,

Could you provide the spec here?

Thanks.

Comment 2

[Evan Tseng [:evanxd]]

We should use building blocks for date, time, text inputs in the add/edit page.

Comment 3

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_edit_event_spec.pdf

Hi Evan,

please check the attachment :)
thanks!

Comment 4

[Harly Hsu[:harly]]

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

Comment 5

[Harly Hsu[:harly]]

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #2)
> We should use building blocks for date, time, text inputs in the add/edit
> page.

After an offline discussion with Arnau, he is now working on connecting calendar input field to Building Blocks style on bug 967592, and he will also connect the value selectors for us.

Comment 6

[Harly Hsu[:harly]]

Currently in calendar, when user go to event detail and decided to edit event, after editing is completed and press done, it should go back to the event detail page instead of month/week/day view. Is it possible to modify it in this visual refresh, or should we open a bug for this?

Comment 7

[Evan Tseng [:evanxd]]

We already filed the Bug 1010135 for the router issue at Comment 6.
Thanks for Harly's kind reminder.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

step 13 of https://moztrap.mozilla.org/manage/case/13038/

Comment 9

[Evan Tseng [:evanxd]]

Hi Peko,

Could you provide the svg file of the "Cancel Button" in the spec doc here?

Thanks.

Comment 10

[Evan Tseng [:evanxd]]

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

WIP patch.

Comment 11

[Peko Chen [:pekochen]]

We could just use this image https://github.com/mozilla-b2g/gaia/blob/master/shared/style/headers/images/icons/back.png.

Comment 12

[Evan Tseng [:evanxd]]

Thanks.
I think you mean that using this image https://github.com/mozilla-b2g/gaia/blob/master/shared/style/headers/images/icons/close.png.

Comment 13

[Evan Tseng [:evanxd]]

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

Hi Harly,

Could you help to review the UI?

Thanks.

Comment 14

[Evan Tseng [:evanxd]]

For the add/update event header, we follow the design of Contacts app.

Comment 15

[Harly Hsu[:harly]]

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

Great work, thank you Evan.

Comment 16

[Evan Tseng [:evanxd]]

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

Hi Miller,

Could you help to review the patch?

Thanks.

Comment 17

[Miller Medeiros [:millermedeiros]]

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

Good work Evan!

Comment 18

[Evan Tseng [:evanxd]]

Thanks for the review, Miller.

Comment 19

[Evan Tseng [:evanxd]]

master: 84b62184b87e229dbb51939a04357ce8c5faa195

Comment 20

[Evan Tseng [:evanxd]]

Sorry, forgot the l10n changes.

Comment 21

[Francesco Lodolo [:flod]]

Not sure if reopening a bug is custom on Gaia, but let's do this, because there's a lot of confusion.

@Evan (and reviewer)

The "late-l10n" keyword is used to warn the l10n team that there are new strings that will land late in the cycle (after string freeze). That's not the case here, since this bug seems to be 2.0 only.

Changing existing strings is just wrong, no point in adding the keyword.
https://developer.mozilla.org/en-US/docs/Making_String_Changes

Changed string = new string ID, unless you're making a minor correction like fixing a typo, and this is not one.

-add-event-header=Event
+add-event-header=Add Event

So please land a patch changing the string ID as soon as possible, since this blocks our process of string extraction.

Comment 22

[Axel Hecht [:Pike]]

PS: late-l10n isn't much of a sign to l10n team, but for release-management to make string freezes, or at least control the lack of

Comment 23

[Evan Tseng [:evanxd]]

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

Hi Francesco,

Thanks for the comments, learned it.

Could you give me feedback for the l10n changes at https://github.com/mozilla-b2g/gaia/pull/19439/files#diff-97b7fed57927556b91fb72d4d1b8c8b5L30? And we could just remove useless l10n items, right?

Thanks.

Comment 24

[Evan Tseng [:evanxd]]

Before I sent the pull request at Comment 23, I already reverted the old patch did the wrong things for 110n.

Comment 25

[Francesco Lodolo [:flod]]

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

Change looks good now.

As a general rule, adding a number to the label should be the last resort: I wonder if in this case we could use "new-event-header", since there's already a similar "new-event" label.

About removing unused strings from .properties files: absolutely yes, they create unnecessary clutter (see also recent bug 1001345 to clean up some files).

Comment 26

[Evan Tseng [:evanxd]]

Hi Francesco,
Sure, let's reuse the 'new-event' label.

Comment 27

[Evan Tseng [:evanxd]]

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

Hi Miller,

Could you review the l10n part of patch again?
See that at https://github.com/mozilla-b2g/gaia/pull/19439/files#diff-97b7fed57927556b91fb72d4d1b8c8b5R8.

Thanks.

Comment 28

[Francesco Lodolo [:flod]]

Wait, I didn't mean to reuse the other strings, that's bad and it took us a while to fix all settings.

What I meant is: call the new string "new-event-header". Never reuse strings in different contexts.

Comment 29

[Miller Medeiros [:millermedeiros]]

Francesco, is there any place where I can read more about Gaia l10n "best practices"? The MDN article about Firefox OS Localization (https://developer.mozilla.org/en-US/Firefox_OS/Developing_Firefox_OS/Localizing_Firefox_OS) doesn't have enough info for developers, it would be really helpful to have a centralized place for all this knowledge. - Not the first time "we" (productivity team) are making this kind of mistakes (I didn't know about half of this stuff before reading your comments). Thanks for all the feedback, learned a lot from you.

Comment 30

[Francesco Lodolo [:flod]]

I don't think we have such a page on MDN. I found this one, but it doesn't cover much
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Writing_localizable_code

Probably time to create such a page, especially now that Firefox OS gets a lot more contributor not familiar with our rules.

Comment 31

[Evan Tseng [:evanxd]]

Hi Miller and Francesco,

I updated the patch for the comments.
Could you help to review the patch?

Learned a lot of l10n things.
Thanks.

Comment 32

[Francesco Lodolo [:flod]]

Looks definitely good to me, thanks.

Comment 33

[Miller Medeiros [:millermedeiros]]

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

Looks good to me! I also learned a lot about L10n. Thanks Francesco!

Comment 34

[Evan Tseng [:evanxd]]

Thanks for the review.

Comment 35

[Evan Tseng [:evanxd]]

master: 9e962fa357001eb5362e30050b83808813fe98a3

Comment 36

[Edward Chen[:edchen]]

[Environment]
Gaia      6a391274cd436f8f0d1fad2db8c6b4805703259c
Gecko     https://hg.mozilla.org/mozilla-central/rev/545c35907eff
BuildID   20140526160203
Version   32.0a1
ro.build.version.incremental=76
ro.build.date=Mon Apr 14 14:02:50 CST 2014

[Result]
Pass