Closed Bug 1337319 Opened 3 years ago Closed 3 years ago

[DateTimePicker] The order of year and month on date picker should depend on locale datetime format

Categories

(Core :: Layout: Form Controls, defect)

54 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: scottwu, Assigned: scottwu)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The order of month and year picker should be locale dependent. For example, the format for en-US should be month-year, but zh-TW should be year-month.
Assignee: nobody → scwwu
Component: XUL Widgets → Layout: Form Controls
Product: Toolkit → Core
Version: unspecified → 54 Branch
This patch uses Intl.DateTimeFormat's formatToParts API to get the order of year and month, then orders the spinners accordingly with `order` in CSS.
Comment on attachment 8838080 [details]
Bug 1337319 - Order month and year spinners based on locale datetime format

https://reviewboard.mozilla.org/r/113074/#review115022

r=me with the below changes. Thanks!

::: toolkit/content/widgets/datepicker.js:328
(Diff revision 2)
>          },
>          getDisplayString: year => yearFormat(new Date(new Date(0).setFullYear(year))),
>          viewportSize: spinnerSize
>        }, context.monthYearView)
>      };
> +    context.monthYearView.classList.add(spinnerOrder);

I wonder if it might be better to put this change _above_ the point where we attach the nodes (so above the construction of the Spinner's), so that we avoid having to recalculate their positions a second time.

::: toolkit/content/widgets/spinner.js:78
(Diff revision 2)
>        this.elements.spinner.style.height = (ITEM_HEIGHT * viewportSize) + "rem";
>  
>        if (hideButtons) {
>          this.elements.container.classList.add("hide-buttons");
>        }
> +      this.elements.container.id = id;

Let's only set this if `id` is not false-y.
Attachment #8838080 - Flags: review?(mconley) → review+
Comment on attachment 8838080 [details]
Bug 1337319 - Order month and year spinners based on locale datetime format

https://reviewboard.mozilla.org/r/113074/#review115022

I've made the changes as suggested. Thanks!

> I wonder if it might be better to put this change _above_ the point where we attach the nodes (so above the construction of the Spinner's), so that we avoid having to recalculate their positions a second time.

Yeah that's a good idea.

> Let's only set this if `id` is not false-y.

Ok. Thanks for catching this.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8b02cdbaa32e
Order month and year spinners based on locale datetime format r=mconley
Keywords: checkin-needed
Hey scottwu - just a heads up that I believe you have the ability to autoland via MozReview. After you've got r+'s, in the "Automation" dropdown, there should be a "Land Commits" option that will attempt to autoland your patches for you. That way, you don't need to set checkin-needed.
https://hg.mozilla.org/mozilla-central/rev/8b02cdbaa32e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.