Bug 107507 – Adjust design of the Calendar Picker

• Component: Forms
• Status: RESOLVED FIXED

[Keishi Hattori]

Adjust design of the Calendar Picker. Use Chrome style buttons. Gray border color. etc.

[Keishi Hattori]

Created attachment 183900 [details]
Patch

[WebKit Review Bot]

Attachment 183900 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 183900 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183900&action=review

> Source/WebCore/Resources/pagepopups/calendarPicker.css:166
> +    height: 24px;

Absolute height is dangerous because -webkit-control font depends on local computer setting.

> Source/WebCore/Resources/pagepopups/calendarPicker.css:175
> +    line-height: 18px;

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:898
> +    disclosureTriangle.innerHTML = "<svg width=\"7\" height=\"5\"><polygon points=\"0,1 7,1 3.5,5\" style=\"fill:#000000;\" /></svg>";

nit: You can avoid ugly escaping by using single quotes in the svg string.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:945
> +    this._left2.innerHTML = "<svg width=\"9\" height=\"7\"><polygon points=\"0,3.5 4,7 4,0\" style=\"fill:#6e6e6e;\" /><polygon points=\"5,3.5 9,7 9,0\" style=\"fill:#6e6e6e;\" /></svg>";

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:950
> +    this._left1.innerHTML = "<svg width=\"4\" height=\"7\"><polygon points=\"0,3.5 4,7 4,0\" style=\"fill:#6e6e6e;\" /></svg>";

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:962
> +    this._right1.innerHTML = "<svg width=\"4\" height=\"7\"><polygon points=\"0,7 0,0, 4,3.5\" style=\"fill:#6e6e6e;\" /></svg>";

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:967
> +    this._right2.innerHTML = "<svg width=\"9\" height=\"7\"><polygon points=\"4,3.5 0,7 0,0\" style=\"fill:#6e6e6e;\" /><polygon points=\"9,3.5 5,7 5,0\" style=\"fill:#6e6e6e;\" /></svg>";

ditto.

> Source/WebCore/Resources/pagepopups/calendarPicker.js:-1018
> -    this._monthPopup.style.left = this._month.offsetLeft + (this._month.offsetWidth - this._monthPopup.offsetWidth) / 2 + "px";
> -    this._monthPopup.style.top = this._month.offsetTop + this._month.offsetHeight + "px";
> +    var bounds = this.picker.daysTableBounds();
> +    this._monthPopup.style.left = bounds.x + "px";
> +    this._monthPopup.style.top = bounds.y + "px";
> +    this._monthPopup.style.width = bounds.width + "px";
> +    this._monthPopup.style.height = bounds.height + "px";
>  
>      this._wall.style.display = "block";
>      this._wall.style.zIndex = "999"; // This should be smaller than the z-index of monthPopup.
>  
> -    var popupHeight = this._monthPopup.clientHeight;
> -    var fullHeight = this._monthPopupContents.clientHeight;
> -    if (fullHeight > popupHeight) {
> -        var selected = this._getSelection();
> -        if (selected) {
> -           var bottom = selected.offsetTop + selected.clientHeight;
> -           if (bottom > popupHeight)
> -               this._monthPopup.scrollTop = bottom - popupHeight;
> -        }
> -        this._monthPopup.style.webkitPaddingEnd = getScrollbarWidth() + 'px';
> -    }

Should we do this change now?  I think this change affects UX, and would be out of scope of this bug (appearance change).

> Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:10
> +.days-area-container:focus {
> +  -webkit-transition: border-color 200ms;

We use 4-space indentation.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:9
> +    inset 0 1px 2px rgba(255, 255, 255, 0.75);

Need additional indentation for this line.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:21
> +:enabled:hover:-webkit-any(button, input[type='button']) {
> +  background-image: -webkit-linear-gradient(#f0f0f0, #f0f0f0 38%, #e0e0e0);

We use 4-space indentation.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:35
> +:disabled:-webkit-any(button, input[type='button']) {
> +  background-image: -webkit-linear-gradient(#f1f1f1, #f1f1f1 38%, #e6e6e6);

ditto.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:43
> +:enabled:focus:-webkit-any(button, input[type='button']) {
> +  -webkit-transition: border-color 200ms;

ditto.

> Source/WebCore/WebCore.gyp/WebCore.gyp:-1350
> -        ['OS=="mac"', {
> -          'include_dirs': [
> -            '<(chromium_src_dir)/third_party/apple_webkit',
> -          ],

You shouldn't remove this block.

[Keishi Hattori]

Created attachment 184121 [details]
Patch

[Keishi Hattori]

Created attachment 184123 [details]
Patch

[WebKit Review Bot]

Attachment 184123 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keishi Hattori]

Comment on attachment 183900 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183900&action=review

>> Source/WebCore/Resources/pagepopups/calendarPicker.js:-1018
>> -    }
> 
> Should we do this change now?  I think this change affects UX, and would be out of scope of this bug (appearance change).

Removing this whole block was a mistake. I just wanted to remove this line.
1017:    this._monthPopup.style.webkitPaddingEnd = getScrollbarWidth() + 'px';

[Keishi Hattori]

Created attachment 184128 [details]
Patch

[WebKit Review Bot]

Attachment 184128 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 184128 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184128&action=review

> Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:10
> +.days-area-container:focus {
> +  -webkit-transition: border-color 200ms;

wrong indentation.

> Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:13
> +  border-color: rgb(77, 144, 254);

should be -webkit-focus-ring-color.
In Chromium Settings page, we respect system's focus ring color.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:44
> +    border-color: rgb(77, 144, 254);

should be -webkit-focus-ring-color.

[Keishi Hattori]

Created attachment 184153 [details]
Patch

[WebKit Review Bot]

Attachment 184153 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 184153 [details]
Patch

Looks good.

[WebKit Review Bot]

Comment on attachment 184153 [details]
Patch

Rejecting attachment 184153 [details] from commit-queue.

New failing tests:
platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html
Full output: http://queues.webkit.org/results/16069003

[WebKit Review Bot]

Comment on attachment 184153 [details]
Patch

Attachment 184153 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16078064

New failing tests:
platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html

[Keishi Hattori]

Created attachment 184187 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184187 [details]
Patch

Rejecting attachment 184187 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'validate-changelog', '--non-interactive', 184187, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

/mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/16072148

[Keishi Hattori]

Created attachment 184188 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184188 [details]
Patch

Rejecting attachment 184188 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'validate-changelog', '--non-interactive', 184188, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

/mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/16065547

[WebKit Review Bot]

Attachment 184188 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-ru-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/month-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-step-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt', u'LayoutTests/platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html', u'ManualTests/forms/calendar-picker.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Resources/pagepopups/calendarPicker.css', u'Source/WebCore/Resources/pagepopups/calendarPicker.js', u'Source/WebCore/Resources/pagepopups/calendarPickerMac.css', u'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css', u'Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css', u'Source/WebCore/WebCore.gyp/WebCore.gyp', u'Source/WebCore/rendering/RenderTheme.cpp', u'Source/WebCore/rendering/RenderTheme.h', u'Source/WebCore/rendering/RenderThemeChromiumMac.h', u'Source/WebCore/rendering/RenderThemeChromiumMac.mm', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/DateTimeChooserImpl.cpp']" exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/calendar-picker/week-picker-appearance-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 184188 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184188&action=review

> ChangeLog:13
> [phone]  Keishi Hattori  <[e-mail]>
> +
> +        Adjust design of the Calendar Picker
> +        https://bugs.webkit.org/show_bug.cgi?id=107507
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * ManualTests/forms/calendar-picker.html:
> +
> [phone]  Keishi Hattori  <[e-mail]>
> +
> +        Adjust design of the Calendar Picker
> +        https://bugs.webkit.org/show_bug.cgi?id=107507

Duplicated ChangeLog entries.

[Keishi Hattori]

Created attachment 184381 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184381 [details]
Patch

Rejecting attachment 184381 [details] from commit-queue.

New failing tests:
platform/chromium/fast/forms/calendar-picker/week-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations.html
Full output: http://queues.webkit.org/results/16073603

[Kent Tamura]

Comment on attachment 184381 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184381&action=review

> LayoutTests/ChangeLog:12
> +        * platform/chromium/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt:
> +        * platform/chromium/fast/forms/calendar-picker/month-picker-key-operations-expected.txt:
> +        * platform/chromium/fast/forms/calendar-picker/month-picker-key-operations.html:
> +        * platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt:

You need to update platform/chromium-win/platform/chromium/fast/forms/calendar-picker/*-picker-key-operations.html too.

We hit this platform difference several times. We might want to make separated tests for F4 key behavior.

[Kent Tamura]

Comment on attachment 184381 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184381&action=review

>> LayoutTests/ChangeLog:12
>> +        * platform/chromium/fast/forms/calendar-picker/week-picker-key-operations-expected.txt:
> 
> You need to update platform/chromium-win/platform/chromium/fast/forms/calendar-picker/*-picker-key-operations.html too.
> 
> We hit this platform difference several times. We might want to make separated tests for F4 key behavior.

operations.html -> operations-expected.txt

[Keishi Hattori]

Created attachment 184653 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184653 [details]
Patch

Rejecting attachment 184653 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 184653, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
ps/calendarPickerMac.css'
patch: **** Can't create file /tmp/ppKjAyJ9 : No space left on device
fatal: pathspec 'Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css' did not match any files
Failed to git add Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 448.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16094015

[Keishi Hattori]

Comment on attachment 184653 [details]
Patch

Clearing flags on attachment: 184653

Committed r140778: <http://trac.webkit.org/changeset/140778>

[Keishi Hattori]

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

Comment on attachment 184653 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184653&action=review

> Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:1
> +.year-month-button {

We should have added a copyright header.

> Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:1
> +input[type='button'],

ditto.

[Kent Tamura]

(In reply to comment #30)
> (From update of attachment 184653 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184653&action=review
> 
> > Source/WebCore/Resources/pagepopups/chromium/calendarPickerChromium.css:1
> > +.year-month-button {
> 
> We should have added a copyright header.
> 
> > Source/WebCore/Resources/pagepopups/chromium/pickerCommonChromium.css:1
> > +input[type='button'],
> 
> ditto.

Added by http://trac.webkit.org/changeset/142359