Closed Bug 1007546 Opened 6 years ago Closed 6 years ago

[Calendar] Add/Edit Event 2.0 Visual Refresh

Categories

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

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.0)

VERIFIED FIXED
2.0 S2 (23may)
feature-b2g 2.0

People

(Reporter: evanxd, Assigned: evanxd)

References

Details

(Keywords: feature, Whiteboard: [priority][p=3])

Attachments

(3 files)

No description provided.
Hi Peko,

Could you provide the spec here?

Thanks.
Flags: needinfo?(pchen)
Target Milestone: --- → 2.0 S2 (23may)
Whiteboard: [priority]
We should use building blocks for date, time, text inputs in the add/edit page.
Hi Evan,

please check the attachment :)
thanks!
Flags: needinfo?(pchen)
Please modify the text "Remind me:" to "Reminder:" to avoid misunderstanding in different languages.
(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.
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?
We already filed the Bug 1010135 for the router issue at Comment 6.
Thanks for Harly's kind reminder.
Status: NEW → ASSIGNED
Hi Peko,

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

Thanks.
Flags: needinfo?(pchen)
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.
Attachment #8424581 - Flags: ui-review?(hhsu)
For the add/update event header, we follow the design of Contacts app.
Comment on attachment 8424581 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19359

Great work, thank you Evan.
Attachment #8424581 - Flags: ui-review?(hhsu) → ui-review+
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.
Attachment #8424581 - Flags: review?(mmedeiros)
Comment on attachment 8424581 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19359

Good work Evan!
Attachment #8424581 - Flags: review?(mmedeiros) → review+
Thanks for the review, Miller.
master: 84b62184b87e229dbb51939a04357ce8c5faa195
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [priority] → [priority][p=3]
feature-b2g: --- → 2.0
Keywords: feature
Sorry, forgot the l10n changes.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Keywords: late-l10n
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Flags: needinfo?(evanxd)
Keywords: late-l10n
Resolution: FIXED → ---
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
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.
Attachment #8425468 - Flags: feedback?(francesco.lodolo)
Flags: needinfo?(evanxd)
Before I sent the pull request at Comment 23, I already reverted the old patch did the wrong things for 110n.
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).
Attachment #8425468 - Flags: feedback?(francesco.lodolo) → feedback+
Hi Francesco,
Sure, let's reuse the 'new-event' label.
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.
Attachment #8425468 - Flags: review?(mmedeiros)
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.
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.
Flags: needinfo?(francesco.lodolo)
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.
Flags: needinfo?(francesco.lodolo)
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.
Looks definitely good to me, thanks.
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!
Attachment #8425468 - Flags: review?(mmedeiros) → review+
Thanks for the review.
master: 9e962fa357001eb5362e30050b83808813fe98a3
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
[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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.