Summary: | Add calendar table view for the new calendar picker | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keishi Hattori <keishi> | ||||||||
Component: | Forms | Assignee: | Keishi Hattori <keishi> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | tkent, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 109439 | ||||||||||
Attachments: |
|
Description
Keishi Hattori
2013-02-27 04:28:17 PST
Created attachment 190493 [details]
Patch
Comment on attachment 190493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190493&action=review > Source/WebCore/Resources/pagepopups/calendarPicker.js:1901 > +DayCell.Height = 20; Is it enough for touch input devices? We use 28px in the pointer:coarse case for now. > Source/WebCore/Resources/pagepopups/calendarPicker.js:2069 > + if (global.params.locale === "ja") { global.params.locale should be getLanguage(). Or, this should check if the region part is "jp" because the rule of blue Saturdays and red Sundays is a common sense in Japan, not Japanese language. (But Google Chrome might provide no "jp" part.) > Source/WebCore/Resources/pagepopups/calendarPicker.js:2204 > + // Disable mouse wheel scrolling. why? We prefer "why" comments to "what" comments. > Source/WebCore/Resources/pagepopups/calendarPicker.js:2329 > + * @return {!Object} should explain the content of Object. Comment on attachment 190493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190493&action=review >> Source/WebCore/Resources/pagepopups/calendarPicker.js:1901 >> +DayCell.Height = 20; > > Is it enough for touch input devices? We use 28px in the pointer:coarse case for now. Can I add touch support in another patch? I'll have to prepare a patch so the ScrollView supports scrolling with touch events so you can scroll in the month popup anyway. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:2069 >> + if (global.params.locale === "ja") { > > global.params.locale should be getLanguage(). > Or, this should check if the region part is "jp" because the rule of blue Saturdays and red Sundays is a common sense in Japan, not Japanese language. (But Google Chrome might provide no "jp" part.) I confirmed that Chrome on Mac only provides "ja" for Japanese. I'll use getLanguage. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:2204 >> + // Disable mouse wheel scrolling. > > why? > We prefer "why" comments to "what" comments. Done. >> Source/WebCore/Resources/pagepopups/calendarPicker.js:2329 >> + * @return {!Object} > > should explain the content of Object. Done. Created attachment 190714 [details]
Patch
Comment on attachment 190493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190493&action=review >>> Source/WebCore/Resources/pagepopups/calendarPicker.js:1901 >>> +DayCell.Height = 20; >> >> Is it enough for touch input devices? We use 28px in the pointer:coarse case for now. > > Can I add touch support in another patch? I'll have to prepare a patch so the ScrollView supports scrolling with touch events so you can scroll in the month popup anyway. Please add a FIXEM comment. Comment on attachment 190714 [details]
Patch
ok
Created attachment 190891 [details]
Patch
Comment on attachment 190891 [details] Patch Clearing flags on attachment: 190891 Committed r144423: <http://trac.webkit.org/changeset/144423> All reviewed patches have been landed. Closing bug. |