Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use quotes around attribute selectors for jQuery Tabs #626

Closed
wants to merge 5 commits into from

3 participants

@bbodien

As per #596

@duclet duclet was assigned
@duclet duclet commented on the diff
src/scrollable/scrollable.js
@@ -141,6 +141,21 @@
return self;
},
+ addItemBefore: function(item) {
+ item = $(item);
+
+ if (!conf.circular) {
+ itemWrap.prepend(item);
+ } else {
+ itemWrap.children("." + conf.clonedClass + ":first").after(item);
+ itemWrap.children("." + conf.clonedClass + ":last").replaceWith(item.clone().addClass(conf.clonedClass));
+ }
+
+ self.seekTo(self.getIndex() +1, 0);
+
+ fire.trigger("onAddItemBefore", [item]);
@duclet Collaborator
duclet added a note

Please change the event name to "onBeforeAddItem" so that it matches the way the other events are named.

@bbodien
bbodien added a note

Shouldn't it then be OnBeforeAddItemBefore? (Event triggered before we add an item before the other scrollable items)

@duclet Collaborator
duclet added a note

You know what, I must have a brain freeze or something. Totally mistook this for what it actually does. Ignore my comment regarding the name change. Though I will note that because this is a new feature, it probably won't be in the next release since I'm hoping for that release to be a bug fix release only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@duclet duclet commented on the diff
src/scrollable/scrollable.js
@@ -186,7 +201,7 @@
});
// callbacks
- $.each(['onBeforeSeek', 'onSeek', 'onAddItem'], function(i, name) {
+ $.each(['onBeforeSeek', 'onSeek', 'onAddItem', 'onAddItemBefore'], function(i, name) {
@duclet Collaborator
duclet added a note

Same note as above.

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

Please follow guidelines here:
https://github.com/jquerytools/jquerytools/wiki/fixing-an-issue

Also, we'll have the fix for the tabs with #679.

@duclet duclet closed this
@u982744

So what happened to adding this feature in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 26, 2010
  1. @bbodien

    create prependItem method for inserting new elements to the start of …

    bbodien authored
    …the scrollable
    
    rename addItem to appendItem for consistency with new method
  2. @bbodien
  3. @bbodien
Commits on Dec 15, 2011
  1. @bbodien
  2. @bbodien
This page is out of date. Refresh to see the latest.
Showing with 17 additions and 2 deletions.
  1. +16 −1 src/scrollable/scrollable.js
  2. +1 −1  src/tabs/tabs.js
View
17 src/scrollable/scrollable.js
@@ -141,6 +141,21 @@
return self;
},
+ addItemBefore: function(item) {
+ item = $(item);
+
+ if (!conf.circular) {
+ itemWrap.prepend(item);
+ } else {
+ itemWrap.children("." + conf.clonedClass + ":first").after(item);
+ itemWrap.children("." + conf.clonedClass + ":last").replaceWith(item.clone().addClass(conf.clonedClass));
+ }
+
+ self.seekTo(self.getIndex() +1, 0);
+
+ fire.trigger("onAddItemBefore", [item]);
@duclet Collaborator
duclet added a note

Please change the event name to "onBeforeAddItem" so that it matches the way the other events are named.

@bbodien
bbodien added a note

Shouldn't it then be OnBeforeAddItemBefore? (Event triggered before we add an item before the other scrollable items)

@duclet Collaborator
duclet added a note

You know what, I must have a brain freeze or something. Totally mistook this for what it actually does. Ignore my comment regarding the name change. Though I will note that because this is a new feature, it probably won't be in the next release since I'm hoping for that release to be a bug fix release only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return self;
+ },
/* all seeking functions depend on this */
seekTo: function(i, time, fn) {
@@ -186,7 +201,7 @@
});
// callbacks
- $.each(['onBeforeSeek', 'onSeek', 'onAddItem'], function(i, name) {
+ $.each(['onBeforeSeek', 'onSeek', 'onAddItem', 'onAddItemBefore'], function(i, name) {
@duclet Collaborator
duclet added a note

Same note as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
// configuration
if ($.isFunction(conf[name])) {
View
2  src/tabs/tabs.js
@@ -276,7 +276,7 @@
});
// open initial tab
- if (location.hash && conf.tabs == "a" && root.find("[href=" +location.hash+ "]").length) {
+ if (location.hash && conf.tabs == "a" && root.find("[href='" +location.hash+ "']").length) {
self.click(location.hash);
} else {
Something went wrong with that request. Please try again.