Select: Add non-native optgroup divider-theme support #4812

Closed
wants to merge 6 commits into
from

2 participants

@MauriceG

@uGoMibo

Hi,
if you have a moment, I'd like you take a look.
Thanks in advance!

Maurice

@jaspermdegroot
jQuery Foundation member

hi @MauriceG

The code looks good to me. I suppose you ran the select unit test and created a test page to see if it works as expected?

There is a small typo in the commit for the docs: the adjusted text should start with "a divider" instead of "divider". That & which was already in that line, looks a bit weird. Do you mind replacing that by "and"?

Can you add the option to the data attribute reference?

Thanks!

@jaspermdegroot
jQuery Foundation member

@MauriceG

I forgot one thing for the docs: "The default theme for group dividers" should be "The default theme swatch for group dividers".

@MauriceG

Hi @uGoMobi !

Sorry for delay. I was on holiday :-)

Thanks for looking into this. I've added some commits for the docs.

Demo at: http://jsfiddle.net/MauriceG/ULZfw/show/light/

Tests: 54 tests of 54 passed, 0 failed.

Hope you and your hardware are back soon!

Maurice

@jaspermdegroot
jQuery Foundation member

hi @MauriceG !

I hope you enjoyed your holiday. I have a new computer so my hardware is all fine now :)

@toddparker - As discussed during last thursday's meeting I will merge this PR for 1.2 beta.

@johnbender @gseguin - Can one of you review the first two commits as well? Thanks!

I will add a unit test for this.

Jasper

@MauriceG MauriceG closed this Sep 12, 2012
@jaspermdegroot
jQuery Foundation member

@MauriceG - Why did you close this PR?

@MauriceG

Hi @uGoMobi

I did close this accidently because I'm a github jackass ...
And I'm unable to reopen it.

Maurice

@jaspermdegroot
jQuery Foundation member

@MauriceG

LOL

I cannot re-open it either because you closed it yourself. New PR? :)

@MauriceG

@uGoMobi

Ok.

  • I've dropped the underlying branch and I did not know, that the PR will be closed then :-/

Maurice

@MauriceG MauriceG added a commit to MauriceG/jquery-mobile that referenced this pull request Sep 19, 2012
@MauriceG MauriceG Select: Add optgroup theming for non-native select - #1
Fix for #4812
438004b
@jaspermdegroot
jQuery Foundation member

Replaced by PR #5033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment