Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed activity to use font awesome spinner. #39

Merged
merged 1 commit into from
Nov 28, 2013
Merged

Fixed activity to use font awesome spinner. #39

merged 1 commit into from
Nov 28, 2013

Conversation

acornejo
Copy link
Contributor

  • Dropped jquery dependency.
  • Added AMD wrapper.
  • Support custom active/inactive classes (for use without font awesome).

This version is much shorter than the original. It also works without font awesome, by allowing users to specify a custom element and class combination to use for signaling active and inactive states.

For instance:

<button class="btn btn-large" data-bind="active: save.isActive, activeOptions: {container: 'span', activityClass: 'busy', inactiveClass: ''}"> Save</button>

This will insert a span container inside the button and apply class 'busy' when the button is active, and no classes when the button is not active.

* Dropped jquery dependency.
* Added AMD wrapper.
* Support custom active/inactive classes (for use without font awesome).
@hfjallemark
Copy link

Just what I had planned, looks great @acornejo. Great job!

hfjallemark pushed a commit that referenced this pull request Nov 28, 2013
Fixed activity to use font awesome spinner.
@hfjallemark hfjallemark merged commit 6c90c88 into CodeSeven:master Nov 28, 2013
@mariusfilipowski
Copy link
Contributor

I really like this simple idea.
It would be cool a addition to change the default values for the activeOptions in one central place.
So everyone who doesn't use "fa fa-spin fa-spinner" as CSS doesn't have to prove his activityOptions to every single binding.

};
}
$(s).remove();
var defaultOptions = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cool to set these options from outside,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fairly easy to do a pullRequest to add that functionality by making defaultOptions accessible from the outside (say, ko.bindingHandlers.activity.defaultOptions).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, like we do in toastr. Send a PR!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR merged.

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

Successfully merging this pull request may close these issues.

3 participants