Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#10974 closed enhancement (duplicate)

[cla][patch] dijit.form._Spinner - Improve CSS - Support padding input fields

Reported by: Jonathan Bond-Caron Owned by: Douglas Hays
Priority: high Milestone: 1.5
Component: Dijit - Form Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

The CSS for the spinner has been improved so that is renders well with FF, IE6+, Opera 9.x+, Safari, Chrome.

An additional bonus is that padding can be added to the 'input':

.dijitSpinner .dijitInputField {
	padding:5px;
}

Is _layoutHackIE7() needed on the spinner? From my tests, I can't reproduce a 'scrolling issue'.

Attachments (16)

spinner_style.patch (6.4 KB) - added by Jonathan Bond-Caron 10 years ago.
arrowCSS.patch (5.0 KB) - added by Jonathan Bond-Caron 10 years ago.
Sample patch, but not working with IE6
dijit.InputField.CSS (19.6 KB) - added by Jonathan Bond-Caron 10 years ago.
First BETA patch for review
dijit.InputField.CSS.patch (24.9 KB) - added by Jonathan Bond-Caron 10 years ago.
First BETA patch for review (full)
dijit.InputField.B2.patch (40.8 KB) - added by Jonathan Bond-Caron 10 years ago.
BETA #2, moderatly well tested, fixes for RTL + a11y
dijit.InputField.B3.patch (44.0 KB) - added by Jonathan Bond-Caron 10 years ago.
BETA 3, this one looks good
SpinnerRightPaddingProblem.jpg (22.6 KB) - added by Douglas Hays 10 years ago.
dijit.InputField.B4.patch (65.5 KB) - added by Jonathan Bond-Caron 10 years ago.
Tested FF 3, IE6+ (quirks + standard) - IE 6 a11y broken
10974_B4_refreshed.patch (43.2 KB) - added by Douglas Hays 10 years ago.
updated B4 patch with #11003 changes to help prevent it from becoming stale, empty SPAN tags in Spinner template removed and invalid icon centered vertically - still need TextBox_sizes.html tests to pass
10974_IE6overflow.jpg (8.0 KB) - added by Douglas Hays 10 years ago.
10974_IE6a11y.jpg (8.6 KB) - added by Douglas Hays 10 years ago.
dijit.InputField.RC1.patch (65.8 KB) - added by Jonathan Bond-Caron 10 years ago.
Hopefully, can commit this patch
TextBox_sizes.patch (4.8 KB) - added by Douglas Hays 10 years ago.
additional textbox rendering tests - currently showing very loose packing in IE6
dijit.InputField.RC2.patch (71.3 KB) - added by Jonathan Bond-Caron 10 years ago.
Well tested
test_ComboBox.png (4.6 KB) - added by bill 10 years ago.
IE6 <input> sizing problem
simplified_a11y_markup_prototype.patch (1.4 KB) - added by bill 10 years ago.
demonstration of one way to simplify markup for a11y

Download all attachments as: .zip

Change History (78)

Changed 10 years ago by Jonathan Bond-Caron

Attachment: spinner_style.patch added

comment:1 Changed 10 years ago by Jonathan Bond-Caron

Small note: patch includes some changes from http://trac.dojotoolkit.org/ticket/10947

comment:2 Changed 10 years ago by Adam Peller

Cc: Douglas Hays added
Component: themesDijit
Owner: nonken deleted

comment:3 Changed 10 years ago by bill

Cc: Douglas Hays removed
Owner: set to Douglas Hays

Jbondc, you've got two different things in the patch. The minimumTimeout thing I think you requested in a different ticket? Anyway it's unrelated to this ticket. (BTW you missed the second "m" in "minimum".)

As for the point of this ticket, about Spinner CSS, is the patch against the latest code? Doug has refactored the form widgets CSS/templates since 1.4, specifically spinner. Also, why is the patch only against soria and nihilo? Thanks.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: arrowCSS.patch added

Sample patch, but not working with IE6

comment:4 Changed 10 years ago by Jonathan Bond-Caron

The first patch was against 1.4.1, I added a new 'sample patch' for trunk.

I saw the nice trick with 'dijitArrowButtonInnerHalfHeight' for IE 6 :)

All modern browsers are smart enough to make the float 'dijitSpinnerButtonContainer' {height 100%} as the container.

I can't think of a way to get this work on IE 6 (including padding height). I'll still work on it though... Any ideas?

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.CSS added

First BETA patch for review

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.CSS.patch added

First BETA patch for review (full)

comment:5 Changed 10 years ago by Jonathan Bond-Caron

So this turned out to be much more work thanks to IE6.

Context: The initial goal was to allow applications to do ".myApp input, .myApp .dijitInputField { padding: 2px }"

So naturally, all input fields are padded.

Information about the patch:

  • Be more consistent.
  • Use the same CSS technique for Spinner and ComboBox?. 'dijitArrowButtonContainer' and 'dijitValidationContainer' are both floats that occupy space. The arrows and validation errors have an *absolute position* wrt the widget container. This is important to give themes more flexibility where they can position those elements.
  • Removed a lot of 'hacks' in the claro theme. From my tests, most of the issues have went away with the new CSS and consistency.
  • Removed a lot of 'forced padding'

i.e .dijitSelect .dijitButtonNode {padding:0px;}

Some known issues:

Patch for 1.5 : I'm really keen on making a well tested patch for 1.5.

The core dijit CSS should allow applications to do for example:

.myApp input, .myApp .dijitInputField { padding: 2px } .myApp button, .myApp .dijitButtonField { padding: 2px } .myApp select, .myApp .dijitSelectField { padding: 2px }

This patch is mainly to get feedback, if this looks good, I can add support for 'dijitButtonField' and 'dijitSelectField'. I will check back at the end of the week.

An extra note: if themes want to add/force padding on elements, they just do: .claro .dijitInputField { padding: 2px } .claro .dijitButtonField { padding: 1.2em } .claro .dijitSelectField { padding: 1.2em !important; }

comment:6 Changed 10 years ago by bill

Description: modified (diff)

Hi Jonathan,

Thanks for working on this, the goals you listed above all sound good. Hopefully Doug can review the work but I'll add my 2 cents too.

The 1.5 beta release is imminent so the timing on this is tight, normally I'd say to push it to 1.6 but OTOH Doug did refactoring of Spinner/ComboBox for the 1.5 release so it would be nice to get all the refactoring into a single release.

You said that claro has quirks on IE6, does that mean it's working worse than before?

Specifics about the patch itself:

claro/Dialog.css:

  • I'll ignore this change, seems unrelated

claro/form/Button.css:

  • we don't want to use !important in theme files (except for dijit.css)

claro/form/Common.css: claro/form/NumberSpinner.css:

  • looks like you made a lot of simplifications, that's good

dijit.css:

  • as you mentioned above, the absolute position of the validation icon allows flexibility in placement, which is nice. But the problem with absolutely positioned validation icon is that it won't work in a11y mode because then the arrow becomes a different width. it's using a character for the icon and the character's size can change based on browser's zoom level (for text-only zoom browsers)
  • dijitValidationContainer will need a float:left in dijit_rtl.css right?

And then some problems I noticed trying it on FF3.6/mac:

  1. in test_validate.html?theme=claro typing something invalid into "Age" field, the validation icon's red background overflows the field's border
  2. in test_validate.html?theme=tundra it doesn't overflow, but the icon isn't centered vertically
  3. test_validate.html?dir=rtl shows the error icon on the right; it should be on the left
  4. test_validate.html?theme=tundra&dir=rtl&a11y=true the validation icon overlaps the input text. Same problem for ComboBox and probably Spinner too.
  5. test_FilteringSelect.html?a11y=true&dir=rtl shows the text getting cutoff by the arrow "icon" (which is really a character because it's high-contrast mode)
  6. test_Spinner.html?theme=claro, when you type error text the validation icon appears (correctly) but the arrow icons disappear (incorrectly). if you do text-only zoom then it looks even stranger.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.B2.patch added

BETA #2, moderatly well tested, fixes for RTL + a11y

comment:7 Changed 10 years ago by Jonathan Bond-Caron

Thanks for the feedback Bill, really happy to hear this can make in 1.5.

The new patch should fix most issues with RTL and a11y.

For ally, I used a fixed font-size for the character. It's also absolutely positioned wrt the the widget container which gives more flexibility to themes.

There's still some issues I'm aware of:

  • Opera 10: The a11y character isn't vertically aligned to the bottom
  • IE 6 : The vertical alignment of inline items isn't working at all, probably need custom CSS hack there...
  • IE 6 : The validation icon is off a bit (used to work but I broke it), something to do with line-height

I took note of:

  • no !important in theme files

The third patch should be the good / well-tested one :)

comment:8 Changed 10 years ago by Douglas Hays

Jonathan, this is a big change. I really appreciate your work on this. With all the template changes, this really needs to go into the 1.5 beta. I ran the following tests in IE6 and there are a lot of failures related to textbox sizes and icon placement that need to be fixed before this patch can be committed:

dijit/tests/form/TextBox_sizes.html?theme=tundra&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=tundra&dir=rtl
dijit/tests/quirks.html?file=form/TextBox_sizes.html&theme=tundra&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=claro&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=claro&dir=rtl
dijit/tests/quirks.html?file=form/TextBox_sizes.html&theme=claro&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=soria&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=soria&dir=rtl
dijit/tests/quirks.html?file=form/TextBox_sizes.html&theme=soria&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=nihilo&dir=ltr
dijit/tests/form/TextBox_sizes.html?theme=nihilo&dir=rtl
dijit/tests/quirks.html?file=form/TextBox_sizes.html&theme=nihilo&dir=ltr

comment:9 Changed 10 years ago by Jonathan Bond-Caron

Thanks, those tests were very useful.

This patch seems to pass all tests on FF 3.6.3, IE 6, IE 7 (quirks & standard mode).

Dijit should probably have some default padding:

.dijitInputField, .dijitSelectField, .dijitButtonField, .dijitTextArea {

padding:0.2em;

}

or

.dijitInputField, .dijitSelectField, .dijitButtonField, .dijitTextArea {

padding:2px;

}

Other then fixing remaining bugs, I might refactor the CSS a little (if time allows).

comment:10 Changed 10 years ago by Douglas Hays

Jonathan, your patch doesn't apply cleanly. It looks like someone changed a bunch of files since yesterday.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.B3.patch added

BETA 3, this one looks good

comment:11 in reply to:  10 Changed 10 years ago by Jonathan Bond-Caron

Replying to doughays:

Jonathan, your patch doesn't apply cleanly. It looks like someone changed a bunch of files since yesterday.

Updated

comment:12 Changed 10 years ago by Douglas Hays

On IE6 with the latest patch, all the above tests render with enornous padding around the input box and the ComboBox? test fails on all themes because of the padding.

comment:13 Changed 10 years ago by Douglas Hays

IE6 + B3 patch + dijit/tests/form/TextBox_sizes.html?theme=tundra&dir=ltr

GROUP "check button sizes" has 2 tests to run     _AssertFailure: [object Error]: assertTrue('false') failed with hint:   ComboBox button height (76) is same as input height (38)

This test passes without the patch.

Changed 10 years ago by Douglas Hays

comment:14 Changed 10 years ago by Douglas Hays

The textbox size test assumes that the padding would go inside the input box to match the button size. Once the padding is reset to normal, then this test may pass but it would be nice to change the test to pass with or without pading so that condition can be tested as well.
However, there is a problem with the Spinner CSS. The Spinner text overlaps the invalid icon and the right padding seems to be missing.

comment:15 Changed 10 years ago by Douglas Hays

The IE test failure is a real failure. The proposed ComboBox? template has a lot of issues:
1) dijitButtonContainer DIV doesn't seem to have any purpose
2) dijitArrowButtonFillHeight DIV is in the template twice
3) SPAN tag contains a block DIV tag which is malformed HTML and we'll be flagged for that
4) empty SPAN node before dijitValidationChar DIV
I hacked up the template and the IE failure went away. I also tested on Safari and the first tundra test at least passes. I didn't test more in case some of my assumptions would cause a problem somewhere else. Here's my revised ComboBox? template:

<div class="dijit dijitReset dijitInlineTable dijitLeft"
        id="widget_${id}"
        dojoAttachPoint="comboNode" waiRole="combobox" tabIndex="-1"
        ><div class="dijitInputLayoutContainer"
                ><div class='dijitReset dijitRight dijitButtonNode dijitArrowButton dijitDownArrowButton'
                                dojoAttachPoint="downArrowNode" waiRole="presentation"
                                dojoAttachEvent="onmousedown:_onArrowMouseDown"
                                ><div class="dijitArrowButtonInner"
                                        ><div class="dijitArrowButtonFillHeight dijitInputField"></div
                                ></div
                                ><div class="dijitInline dijitArrowButtonChar">&#9660;</div
                        ></div
                ><div class="dijitReset dijitValidationContainer"
                        ><BR><div class="dijitReset dijitValidationIcon"><div class="dijitInputField"></div></div
                        ><div class="dijitReset dijitValidationIconText"><div class="dijitInline dijitValidationChar">&Chi;</div></div
                ></div
                ><div class="dijitReset dijitInputField"
                        ><input ${!nameAttrSetting} type="text" autocomplete="off" class='dijitReset'
                        dojoAttachEvent="onkeypress:_onKeyPress,compositionend"
                        dojoAttachPoint="textbox,focusNode" waiRole="textbox" waiState="haspopup-true,autocomplete-list"
                /></div
        ></div
></div>

I also noted that the invalid icon is positioned at 95% of the way to the bottom. Is this intentional? I rather liked center but that's just my opinion.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.B4.patch added

Tested FF 3, IE6+ (quirks + standard) - IE 6 a11y broken

comment:16 in reply to:  15 Changed 10 years ago by Jonathan Bond-Caron

Replying to doughays:

The IE test failure is a real failure. The proposed ComboBox? template has a lot of issues:
1) dijitButtonContainer DIV doesn't seem to have any purpose

Did you mean 'dijitArrowButtonContainer'? It's actually important, a float that occupies space. You might not have noticed because of "right padding 20px" on inputField.

2) dijitArrowButtonFillHeight DIV is in the template twice
3) SPAN tag contains a block DIV tag which is malformed HTML and we'll be flagged for that
4) empty SPAN node before dijitValidationChar DIV

I'll look into this, the new patch might still have these issues.

I also noted that the invalid icon is positioned at 95% of the way to the bottom. Is this intentional? I rather liked center but that's just my opinion.

It's intentional so that the tooltip box points to the error icon. The error icon aligned with the input text (centered) is also ok.

About the new patch B4

  • It should pass all tests EXCEPT *I6 a11y*
    tests/form/TextBox_sizes.html?a11y=true
  • Added default padding 4px
  • Added new test case: dijit\tests\form\test_background.html

I broke something *again* on IE6. I have an idea on how to fix but trying to simplify the markup / make it consistent.

comment:17 Changed 10 years ago by Douglas Hays

With B4, the ComboBox? template still has an empty span tag, and ComboButton?'s arrow side seems narrower than it was in tundra previously. I didn't check all the themes.

comment:18 Changed 10 years ago by Douglas Hays

Jonathon, we want to get rid of the BUTTON nodes in all various the Button widgets since it seems to cause browser-specific rendering issues. Is this something that you could help work on? I'm going to go through and add hidden INPUT nodes to the templates so that FORM submits still work.

comment:19 Changed 10 years ago by Douglas Hays

[21894] changed the Button templates by adding a hidden INPUT tag and moving the onclick dojoAttachPoint to the sibling BUTTON tag. #11003 is being used to convert the BUTTON tags to SPAN tags in the templates.

comment:20 Changed 10 years ago by Douglas Hays

Milestone: tbd1.5

comment:21 Changed 10 years ago by Douglas Hays

[21895] changes the BUTTON tags to SPANs.

Changed 10 years ago by Douglas Hays

Attachment: 10974_B4_refreshed.patch added

updated B4 patch with #11003 changes to help prevent it from becoming stale, empty SPAN tags in Spinner template removed and invalid icon centered vertically - still need TextBox_sizes.html tests to pass

Changed 10 years ago by Douglas Hays

Attachment: 10974_IE6overflow.jpg added

comment:22 Changed 10 years ago by Douglas Hays

Jonathon, I'm testing with the refreshed B4 patch on the latest trunk using IE6 and TextBox_sizes.html?theme=tundra&dir=ltr and the FilteringSelect? and NumberSpinner? textboxes are overflowing through the right padding and onto the invalid icon area.

Changed 10 years ago by Douglas Hays

Attachment: 10974_IE6a11y.jpg added

comment:23 Changed 10 years ago by Douglas Hays

with IE6 in a11y mode, the FilteringSelect?'s validation icon and down arrow placement seem a little off, as compared with the NumberSpinner? that's below it.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.RC1.patch added

Hopefully, can commit this patch

comment:24 Changed 10 years ago by Jonathan Bond-Caron

Hi Doug,

I missed your patch update, the patch I just submitted is tested on IE6, IE7, FF 3, Opera 10:

dijit/tests/form/TextBox_sizes.html dijit/tests/form/TextBox_sizes.html?a11y=true dijit/tests/quirks.html?file=form/TextBox_sizes.html?a11y=true dijit/tests/quirks.html?file=form/TextBox_sizes.html&dir=rtl

Is it good enough to be committed?
If it passes all tests but there are remaining issues, we can open individual tickets.

The markup could be a lot simpler if it weren't for IE6.

comment:25 Changed 10 years ago by Douglas Hays

It looks like dijit/tests/form/test_button.html?theme=tundra ComboButton? widgets have the hover style applied all the time.

comment:26 Changed 10 years ago by Douglas Hays

I think it can be committed as soon as the tundra/ComboButton unhovered-style is fixed.

comment:27 Changed 10 years ago by bill

I did some spot checking on that RC1 patch and I'm seeing a few serious issues:

  • on FF3.6/mac or windows, test_validate.html?theme=claro&dir=rtl, the validation icon for the age field is in the middle of the box
  • on FF3.6/win test_validate.html?theme=claro&a11y=true, after doing ctrl++ the X disappears (my View/Zoom? submenu has "zoom text only" checked)
    • in test_Button.html in high contrast mode on IE8, and also IE8/IE7compat mode, the text completely overflows the buttons (for everything but ComboButton)

I also noticed a problem where the caret in a TextBox isn't visible in high-contrast mode with dir=rtl (on IE8), but that's unrelated to this patch.

comment:28 Changed 10 years ago by Douglas Hays

Jonathon, can you fix these? They all seem to be major regressions associated with your patch.

comment:29 Changed 10 years ago by Jonathan Bond-Caron

Thanks,

test_validate.html?theme=claro&dir=rtl

Fixed

test_validate.html?theme=claro&a11y=true

Fixed "zoom text only"

test_Button.html

Fixed, this was pretty bad!

.dijit_a11y .dijitButtonNode {

width:18px;

}

should have been:

.dijit_a11y .dijitArrowButton {

width:18px;

}

I'll test again and send an updated patch tomorrow.

comment:30 Changed 10 years ago by bill

OK thanks, hopefully the next patch will be the one. Don't forget to test themeTesterQuirk.html and themeTesterQuirk.html?dir=rtl on IE6. I haven't tried them but often quirks mode has problems.

Changed 10 years ago by Douglas Hays

Attachment: TextBox_sizes.patch added

additional textbox rendering tests - currently showing very loose packing in IE6

comment:31 Changed 10 years ago by Douglas Hays

I attached a patch to TextBox_sizes.html that I'd like to commit with this ticket. The patch adds more robust rendering checks. Looks good in FF3.6 and Safari4, not so much with IE6 which has a lot of whitespace within the TextBox? widgets between individual subcomponents.

Changed 10 years ago by Jonathan Bond-Caron

Attachment: dijit.InputField.RC2.patch added

Well tested

comment:32 Changed 10 years ago by Jonathan Bond-Caron

As the tests say "WOOHOO!"

I think we have a winner, a couple of notes about this patch:

  • Passes all tests (that I tried)
  • Fixed bad arrow vertical align in Quirks mode:

http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/quirks.html?file=form/test_Button.html

  • IE 6 has a 3px margin between input field & validation icon

I changed TextBox_sizes.html to allow this, it's an acceptable bug. I don't have any workaround for it.

  • dijit/tests/form/test_Select.html

'dijitSelectFixedWidth' not padding friendly.

Don't have any solution for this at the moment, should a open NEW ticket. Can fix for 1.5 release.

Some possible enhancements:

.dijit_a11y .dijitButtonIcon {  /* Hide icons in a11y */
   display:none;
}
.dijitInline {
  vertical-align: text-bottom;  /* more consistent across browsers, follows more CSS default of 'baseline' */
}

A little outdated but still useful: http://www.zipcon.net/~swhite/docs/computers/browsers/table_vertical_align.html

comment:33 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [21926]) Proxy commit for jbondc. Fixes #10974. Refactor dijit/form templates to make custom styles easier to create. Made TextBox_sizes.html automated tests more robust to check for additional CSS problems.

comment:34 Changed 10 years ago by Douglas Hays

#11029 opened to address a11y ComboButton? problem

comment:35 Changed 10 years ago by bill

It seems to be working pretty well. :-)

It's disappointing that the templates ballooned up so much, and I wonder if we really need all that markup. Yes, I know you added lots for IE6 but I still hope it can be reduced some. Particularly a11y related code, for example:

><div class="dijitArrowButtonText"
	><div class="dijitInline"><div class="dijitArrowButtonFillHeight dijitInputField"></div></div
	><span class="dijitArrowButtonChar">&#9660;</span
></div

and

><div class="dijitReset dijitValidationContainer"
	><BR><div class="dijitReset dijitValidationIcon"><div class="dijitInputField"></div></div
	><div class="dijitReset dijitValidationIconText"><div class="dijitInline"><div class="dijitInputField"></div></div><span class="dijitValidationChar">&Chi;</span></div
	></div

We aren't going for pixel-perfect layout in high-contrast mode, just something that's usable. Hopefully with the "icon" centered vertically but that's not worth spending too much time on. And we can also make simplifications like hardcoding the amount of padding in dijit_a11y mode (probably to 0).

I'm not sure why the 3px gap between the validation icon and the buttons is unfixable, seems like an IE hack (margin: -3px etc.) would work around it. I guess it's not a serious issue though.

One minor thing I noticed is that the border around the [x] validation icon character is missing. Apparently you changed it to be white text on a black background, but that's useless since high contrast mode only supports two colors, and both color and background-color CSS settings are ignored.

About:

.dijit_a11y .dijitButtonIcon {  /* Hide icons in a11y */
   display:none;
}

I agree that's an improvement although I'd like to hide all icons in a11y mode, not just button icons, since they don't show up anyway.

About:

.dijitInline {
  vertical-align: text-bottom;  /* more consistent across browsers, follows more CSS default of 'baseline' */
}

That's interesting. I'll try it out.

comment:36 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

PS: there's another issue that needs to be resolved on IE6: in ComboBox (and probably Spinner too) the <input> is stretching too far, going underneath the arrow(s). It's easy to see w/IE DOM Inspector or if you just keep typing until the blinking caret is on top of the arrow. That needs to be fixed because the designer may have set text-align:right (often used for numbers), and also just in case the user types in a string that's longer than the input box, the horizontal scrolling won't work correctly.

I was playing with the template for ComboBox, trying to simplify the markup used for a11y support for the arrow. I came up with:

><div class="dijitArrowButtonImg"
	><div class="dijitArrowButtonFillHeight dijitInputField"></div
	></div
><div class="dijitArrowButtonText" style="margin-top: 30%;">&#9660;</div

That was working everywhere but IE6; I'm hoping that IE6 problem is due to the too-long <input>.

comment:37 in reply to:  36 Changed 10 years ago by Jonathan Bond-Caron

Replying to bill:

PS: there's another issue that needs to be resolved on IE6: in ComboBox (and probably Spinner too) the <input> is stretching too far, going underneath the arrow(s). It's easy to see w/IE DOM Inspector or if you just keep typing until the blinking caret is on top of the arrow. That needs to be fixed because the designer may have set text-align:right (often used for numbers), and also just in case the user types in a string that's longer than the input box, the horizontal scrolling won't work correctly.

Which test is this? The TextBox_sizes.html test checks for this. No problem I can see...

I was playing with the template for ComboBox, trying to simplify the markup used for a11y support for the arrow. I came up with:

><div class="dijitArrowButtonImg"
	><div class="dijitArrowButtonFillHeight dijitInputField"></div
	></div
><div class="dijitArrowButtonText" style="margin-top: 30%;">&#9660;</div

That was working everywhere but IE6; I'm hoping that IE6 problem is due to the too-long <input>.

Well unfortunately IE6 is *the* problem.

Try this in I6:

<div style="position:relative; padding:20px; width:300px">

<div style="position:absolute; right:0; top:0; width:5px; height:100%; background-color:#000"></div>

</div>

All modern browsers will make the absolute 100% of the height, not IE 6. All the hacks are so that in IE6, the relative div actually grows to 100%.

So doing margin-top: 30%; can't work because the div doesn't know its height 100%.

That's if dijit uses the absolute/relative strategy which I think is better moving forward. Hopefully in 1-2 years, the nasty IE6 markup will go away.

comment:38 Changed 10 years ago by Douglas Hays

Bill, I don't see the overflow-input problem in IE6. What test file/theme/dir/a11y are you using?

comment:39 in reply to:  35 Changed 10 years ago by Jonathan Bond-Caron

Replying to bill:

One minor thing I noticed is that the border around the [x] validation icon character is missing. Apparently you changed it to be white text on a black background, but that's useless since high contrast mode only supports two colors, and both color and background-color CSS settings are ignored.

I inverted the colors initially to see if it was using 100% of the height, then thought it looked quite nice.

What's the right way to test high contrast mode / so that I only see 2 colors?

comment:40 Changed 10 years ago by bill

Hmm, it's strange that you guys don't see it, it's on test_ComboBox.html, in either tundra or claro:

IE6 <input> sizing problem

I also thought that the sizing test would catch the problem too, but I think it's missing it because the problem goes away when the validation icon is displayed. (I can see the problem in !TextBox_sizes.html too if I manually select Kentucky from the drop down.)

About high-contrast mode:

  1. use a Windows machine
  2. control panel / accessibility options / display --> check the "use high contrast" checkbox

Finally, about IE6 markup, I don't want to have special IE6 markup just to make high-contrast mode look nice there. So let's fix the <input> problem first and then see if we can at least simplify the markup for high-contrast mode, even if it means that the buttons aren't centered correctly (in high contrast mode).

Changed 10 years ago by bill

Attachment: test_ComboBox.png added

IE6 <input> sizing problem

comment:41 Changed 10 years ago by Jonathan Bond-Caron

Good catch,

It's the combobox only, input sizing fixed in this patch:

http://trac.dojotoolkit.org/attachment/ticket/11029/a11y_button.patch

comment:42 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [21935]) Fix width on <input> in ComboBox on IE6 (previously it was too wide, overflowing into the arrow). Patch from jbondc (CLA on file), thanks!. Fixes #11029 and #10974 !strict.

Changed 10 years ago by bill

demonstration of one way to simplify markup for a11y

comment:43 Changed 10 years ago by bill

Thanks for the patch. I attached a prototype demonstrating how the a11y related markup can be simplified. This just does it for ComboBox's arrow, but can you try to do it across all the templates that you've changed, for both arrows and validation icons?

comment:44 Changed 10 years ago by Douglas Hays

(In [21938]) References #10974. Add automated tests to catch textbox size problem fixed by [21935].

comment:45 Changed 10 years ago by Douglas Hays

(In [21939]) References #10974. ComboButton? uses a TABLE and in IE it needs a vertical-alignment default of text-bottom to match other button widgets.

comment:46 Changed 10 years ago by Douglas Hays

jbondc, if you go with Bill's simplified-a11y patch, I think we should use dijitArrowButtonChar and not dijitArrowButtonText as the class name for better backward compatibility.

comment:47 Changed 10 years ago by Douglas Hays

Of course we would want the TextBox_sizes.html tests to pass and Bill's a11y prototype template has a failure since the ComboBox? button is too tall.

comment:48 in reply to:  43 Changed 10 years ago by Jonathan Bond-Caron

Replying to bill:

Thanks for the patch. I attached a prototype demonstrating how the a11y related markup can be simplified. This just does it for ComboBox's arrow, but can you try to do it across all the templates that you've changed, for both arrows and validation icons?

Good attempt but it just won't work at all... at least to pass the tests in TextBox_sizes.html. The height: 5em depends on the font-size.

As soon as you start changing font-size + padding, this will break in IE 6, even to a point where the arrows won't be displayed.

I come back to this example for IE6:

<div style="position:relative; padding:20px; width:300px; font-size:100%;">

<div style="position:absolute; right:0; top:0; width:5px; background-color:#000"></div>

</div>

If you can find a magical way for the inner div to occupy 100% of the height or be centered then the markup can be simplified.

One possible simplification in IE6 is to have the validation icon:

<div class="dijitReset dijitValidationChar">&Chi;</div>

The icon would be shown at the top but TextBox_sizes.html would need to allow this since it's not 100% of the height.

comment:49 Changed 10 years ago by Douglas Hays

jbondc/bill, playing with the ComobBox? template, I changed

><div class="dijitReset dijitValidationIconText"><div class="dijitInline"><div class="dijitInputField"></div></div><span class="dijitValidationChar">&Chi;</span></div

to

><div class="dijitReset dijitValidationIconText"><div class="dijitInputField dijitValidationChar"
style="vertical-align:0px;"
>&Chi;</div></div

for a 2 node reduction in the a11y case. I manualy added the vertical-align:0px which would of course go into the dijitValidationChar rule if we like this change.
Comments?

comment:50 Changed 10 years ago by bill

Jonathan:

Good attempt but it just won't work at all... at least to pass the tests in !TextBox_sizes.html.

Changing the a11y markup has no effect whatsoever on the !TextBox_sizes.html test because !TextBox_sizes.html intentionally does not test a11y mode. I've run the tests (with my ComboBox template change) and they work perfectly, have you tried them? Using this markup for the a11y arrow:

><div class="dijitArrowButtonText" style="margin-top: 30%;">&#9660;</div

As soon as you start changing font-size + padding, this will break in IE 6, even to a point where the arrows won't be displayed.

The are displayed perfectly, I am looking at them on my screen right now, on IE6 (with the ComboBox template change I listed above).

If you can find a magical way for the inner div to occupy 100% of the height or be centered then the markup can be simplified.

Again, centering and 100% are not required for a11y mode.

Doug:

Your validation-icon change is similar to the change I suggested above. It's definitely better than what we have now, although it still has one more node than necessary. Not sure what the vertical-align:0px is for, it seems to work without that.

BTW thanks for adding the tests in [21938].

comment:51 Changed 10 years ago by bill

OK, I see that in [21926] you changed module.js to explicitly call !TextBox_sizes.html?a11y=true. But !TextBox_sizes.html is checking many things that we don't enforce in a11y mode, specifically:

  • equal heights of input, validation icon, and arrows
  • equal sizes for spinner, combobutton, and textbox
  • no gaps between input, validation icon, and arrows

(The answer to the comment previously in module.js: "is there a UI contract that a11y textbox widgets have consistent heights and widths?" is no.)

If possible, it's fine to check that the arrows are displayed properly and that none of the subcomponents overlap each other, but I don't want to add markup to all the templates just to make that test pass in a11y mode. Even though those added nodes are display:none, they will slow down performance.

So, let's either stop running the tests in a11y mode, or modify them so they aren't as strict. And then slim down the templates.

comment:52 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

Also [21926] caused serious issues with Select on claro, it's missing borders, has the wrong background color, has more padding then it is supposed to, and it has rounded corners when it shouldn't. It seems tied to the fact that you added dijitButtonNode as a class on the Select's domNode, maybe that shouldn't be there.

comment:53 Changed 10 years ago by bill

(In [21959]) attempt to fix regressions in Select widget on claro theme (introduction of rounded corners, missing inner border, wrong background color, too much padding) from [21926], refs #10974.

comment:54 Changed 10 years ago by bill

(In [21960]) regression from [21926], placeHolder was appearing in upper/left part of browser window, not inside TextBox. Refs #10974, #3286.

comment:55 Changed 10 years ago by Jonathan Bond-Caron

Can this issue be closed and open a new ticket called "simplify form templates"?

To be honest, I'm frustrated to hear that ~ "a11y doesn't need tests" especially after spending hours trying to make it pass for IE6.

The claro select issue is caused by #11029

comment:56 in reply to:  51 Changed 10 years ago by Jonathan Bond-Caron

Replying to bill:

Even though those added nodes are display:none, they will slow down performance.

So, let's either stop running the tests in a11y mode, or modify them so they aren't as strict. And then slim down the templates.

I can work on slimming down the templates but let's define what's acceptable in terms of tests.

A new ticket please with what dijit needs :)

comment:57 Changed 10 years ago by Douglas Hays

#11034 opened to address the TextBox? template node count

comment:58 Changed 10 years ago by bill

(In [21982]) trying to get button padding equal to how it looked before [21926], refs #10974.

comment:59 Changed 10 years ago by bill

(In [21984]) temporarily rolling back most of the Select changes from [21926], until we can get back to the same look & feel as before. refs #10974.

comment:60 Changed 10 years ago by bill

(In [21998]) Rolling back changes from #10974 and #11029. This amounts to rolling back to [21926] (for the files changed in [21926]) and then adding back [21981] and [21983].

Concerning the goal of #10974, to make it easy to set padding on form input widgets:

  • That will be implemented in the textbox widgets (*TextBox, ComboBox/FilteringSelect, Spinner) in a different way, see #11034.
  • I think likewise for the <table> based widgets, Select and ComboButton, since centering arrows in a <table> should be easy, regardless of padding.
  • As for DropDownButton, not sure, but rolling back that change for now.

!TextBox_sizes.html can be restored to [21938] once #11034 is fixed.

Note that many of the changes in [21926] (and reversions in this check in) were due to changing the clas name dijitArrowButtonInner to dijitArrowButtonImg. For back-compat reasons I'd rather not have a unnecessary CSS name change.

Refs #10974, #11029. Fixes #11047.

comment:61 Changed 10 years ago by bill

Resolution: duplicate
Status: reopenedclosed

I'm going to close this ticket in favor or #11034. After #11034 is fixed, feel free to open a new ticket regarding issues in the non-textbox widgets (button, spinner, etc.). As I said above though I think we should have a different approach to handling the <table> based widget's than what was in the original checkin for this ticket.

comment:62 Changed 9 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.