Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

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

Closed
wants to merge 6 commits into from
Closed

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

wants to merge 6 commits into from

Conversation

MauriceG
Copy link
Contributor

@MauriceG MauriceG commented Aug 7, 2012

@uGoMibo

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

Maurice

@jaspermdegroot
Copy link
Contributor

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
Copy link
Contributor

@MauriceG

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

@MauriceG
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

@MauriceG - Why did you close this PR?

@MauriceG
Copy link
Contributor Author

Hi @uGoMobi

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

Maurice

@jaspermdegroot
Copy link
Contributor

@MauriceG

LOL

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

@MauriceG
Copy link
Contributor Author

@uGoMobi

Ok.

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

Maurice

@jaspermdegroot
Copy link
Contributor

Replaced by PR #5033

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants