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

I added support for passing a function as input to the selectClass option #295

Closed
wants to merge 5 commits into from

Conversation

ericfreese
Copy link

So you can pass a function into the selectClass option. Pass $el as 'this' to the function to allow context-aware class definitions.

So you can do something like this:

$('select').uniform({
  selectClass: function() {
    var cls = this.data('uniform-class');
    return (cls !== undefined && cls.length > 0 ? 'selector ' + cls : 'selector');
  }
});

What do you think?

So you can pass a function into the selectClass option. Pass $el as 'this' to the function to allow context-aware class definitions.

For example:

  $('select').uniform({
    selectClass: function() {
      var cls = this.data('uniform-class');
      return (cls !== undefined && cls.length > 0 ? 'selector ' + cls : 'selector');
    }
  });
@fidian
Copy link
Collaborator

fidian commented Feb 19, 2013

Why would you want this functionality instead of just specifying a class on the element itself?

<select class="something">

Then you could specify

.selector.something { ... }

in your CSS without needing to call a special callback.

@ericfreese
Copy link
Author

I chose that approach to be as unobtrusive and flexible as possible.

After grabbing the class from the select element and adding it to the div.selector, you can either remove the class from the original select element, or you can leave it on. If you remove it, you could be preventing someone from (for whatever reason) wanting a class to be applied to the original hidden select element. If you leave it on, it will have the same class that div.selector has, which could lead to style conflicts.

This dilemma can be avoided by using some other semantic attribute such as data-uniform-class. You can leave it on the original select element with no repercussions.

However, it might make sense to make uniform look for the data-uniform-class, and apply the class to div.selectorautomatically. This would standardize the behavior and reduce the need to duplicate the callback across multiple $.uniform calls.

Something along the lines of:

divClass: ($el.data("uniform-class") !== undefined ? options.selectClass + " " + $el.data("uniform-class") : options.selectClass),

@fidian
Copy link
Collaborator

fidian commented Feb 20, 2013

I guess I'm stuck with the question of why this is still necessary? Is it not possible to add the required class in the HTML or in JavaScript before or after using Uniform? Or perhaps by picking that element and using a special class instead by setting options? Could you describe a scenario where this would apply? I'd be happy to explain the use of the above approaches in your scenario to see if that would fit you as well.

@ericfreese
Copy link
Author

The end goal for me is to be able to support multiple styles of select dropdowns, specifically multiple sizes. I'd like to be able to add classes to my select elements in the original markup and have those classes reflected in the markup created by uniform. See this fiddle for an idea of what I'm trying to do: http://jsfiddle.net/egDPJ/

Is it not possible to add the required class in the HTML or in JavaScript before or after using Uniform?

I could write some javascript to do this after calling uniform, looking for any select elements with data-uniform-class set and applying those classes to the ancestor div.selector, but then I'll have to include that javascript everywhere I include a call to uniform, which increases duplication and makes maintenance more difficult. It would be nice if uniform would support it out of the box.

@fidian
Copy link
Collaborator

fidian commented Feb 20, 2013

I've already tackled a similar problem by allowing for easier multi-theme support in Uniform. The end result of it has looked a bit like this:

<select class="secondTheme"></select>
<select class="thirdTheme"></select>
<select></select>
var everything = $('select, input, textarea');
everything.filter('.secondTheme').uniform({ wrapperClass: "secondTheme" });
everything.filter('.thirdTheme').uniform({ wrapperClass: "thirdTheme" });
everything.uniform();

This results in something vaguely like this:

<div class="selector secondTheme"><span><select class="secondTheme"></select></span></div>
<div class="selector thirdTheme"><span><select class="thirdTheme"></select></span></div>
<div class="selector"><span><select></select></span></div>

@fidian
Copy link
Collaborator

fidian commented Feb 20, 2013

I should mention that this is on the develop branch.

Your solution isn't very generic - it only applies to the span and it only applies to selects. I could easily make the concept work for most of the other classes everywhere else, but I am concerned about the perceived need of this by everyone else that uses it.

Another feature request might encompass what you need - #80 asks for inheriting classes from the elements, which would be another way to get this done. You'd add a class to the select and the <div> would get both .selector and your classes. Perhaps this sort of functionality would solve your issue too, and we should focus work towards that goal?

…ginal input elements

Applies to:
 - select (non-multiline)
 - radio
 - checkbox
 - file input

Has the side effect that there is now no way to specify a class to be left on the original select or input element.

A few possible solutions:
 - Leave the class on the element all the time
    - Could result in style conflicts (i.e. styling meant for wrapper div also applies to hidden select/input element.
 - Add an option to either leave the class on or remove it.
 - Take the classes off, but rename them with "uniform-" prefix, similar to how id's are inherited.
 - Use a data attribute such as "data-uniform-class" to specify classes that should be added to the wrapper div
    - Wouldn't work with jQuery < 1.4.3
    - Would require different class definitions for regular inputs that don't use wrapper divs and inputs that do (i.e. class vs. data-uniform-class)
@fidian
Copy link
Collaborator

fidian commented Feb 26, 2013

Great start!

Would you be willing to do the following?

  • Make this optional (via the per-element "options")
  • Allow the preservation of the class
  • Also make this work with $.uniform.restore() and $.uniform.update()

The idea was to also be able to inherit "error" classes so I could use a validation library to flag fields with an error class and then copy/move that class up to the wrapper so I can change themes or style the div appropriately with some sort of error state.

@ericfreese
Copy link
Author

Let me just clarify and make sure I'm implementing the right thing.

The use case for $.uniform.update() is:

  • Add a class to a uniformed input
  • Call update
  • The classes you added to the input element are added to the wrapper div

Is that right?

Should the classes added via update be added back to the input when you call restore?

In other words, if you have:

<select class="amazing-class"></select>
$('select').uniform();

You get

<div class="selector amazing-class">
    <span>whatever</span>
    <select></select>
</div>

Then

$('select').addClass('error');
$.uniform.update('select');

You get

<div class="selector amazing-class error">
    <span>whatever</span>
    <select></select>
</div>

Then

$.uniform.restore('select')

Do you then have

<select class="amazing-class error"></select>

or just

<select class="amazing-class"></select>

?

@fidian
Copy link
Collaborator

fidian commented Mar 5, 2013

I would leave the classes alone on the element and just migrate them up to the div or to the wrapper class in the case of textarea, input, and multi-select. The less one messes with the elements, the better.

Restore is supposed to remove all trace of Uniform and return your HTML back to whatever state it would be in normally had you not applied Uniform. In this case, leaving the classes on the elements would also make restore easier.

Also, don't forget that only some classes should migrate. I don't know about what a reasonable default would be in this case. Perhaps migrate all classes unless there are whitelist/blacklist rules passed in as options? I'd also like to allow patterns. Something like this totally non-working pseudocode:

matches = $element.getClasses();
excluded = [];

if (options.blacklist) {
    moveMatches(blacklist, matches, excluded);  // Move classes from matches to excluded
    if (options.whitelist) {
        moveMatches(whitelist, excluded, matches); // Whitelist some back
    }
} else if (options.whitelist) {
    excluded = matches;
    matches = [];
    moveMatches(whitelist, excluded, matches);  // Only keep ones in whitelist
}
// now copy the classes in matches to the parent

You'll have to be really careful to notice when classes get removed from the element and then $.uniform.update() is called.

@shehi
Copy link
Member

shehi commented Apr 6, 2016

Please resolve conflict issues and resend the PR for further investigation. Can't tell from diff what is conflicting. I will close this PR if no response comes in a month's period.

@shehi shehi closed this Apr 7, 2016
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.

3 participants