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

More config params + only append style tag once #41

Merged
merged 3 commits into from
Apr 25, 2017
Merged

More config params + only append style tag once #41

merged 3 commits into from
Apr 25, 2017

Conversation

andrewchilds
Copy link
Contributor

Hello! This PR adds three things:

  1. Adds a defaultPosition option (useful if the bound element uses something other than position: relative by default).
  2. Adds a namespace option, to make it possible to unbind elements (this is useful if we want to re-render bound elements). This fixes issue Realtime enable and disable #31.
  3. Only append the style tag to the HEAD if it doesn't already exist. Otherwise we end up with a duplicate style tag for every time this library is initialized.

Let me know if you need anything further to get this merged.

Cheers!
Andrew

@andrewchilds
Copy link
Contributor Author

andrewchilds commented Apr 24, 2017

And just for reference, to unbind bound elements, we do something similar to...

var LIBRARY_NAMESPACE = 'TSS';
var LIBRARY_CLASSNAME = '.theiaStickySidebar';

function destroy(boundElement) {
  // Unbind scroll / resize event handlers:
  $(window).off('resize.' + LIBRARY_NAMESPACE);
  $(document).off('scroll.' + LIBRARY_NAMESPACE);

  // Remove style tag:
  $('#theia-sticky-sidebar-stylesheet-' + LIBRARY_NAMESPACE).remove();

  // Remove wrapper element:
  boundElement = $(boundElement);
  boundElement.removeAttr('style').html(boundElement.find(LIBRARY_CLASSNAME).html());
}

function destroyAll() {
  $(LIBRARY_CLASSNAME).parent().each(function (i, element) {
    exports.destroy(element);
  });
};

@liviucmg
Copy link
Member

Thank you @andrewchilds and @webfella, very good points all around. 🍻 Not sure if the destroy logic should be built-in as well, but it's a great start. I'll merge this, re-compile the dist files, and finally bump the version number.

@liviucmg liviucmg closed this Apr 25, 2017
@liviucmg liviucmg reopened this Apr 25, 2017
@liviucmg liviucmg merged commit f3e9028 into WeCodePixels:master Apr 25, 2017
@andrewchilds andrewchilds deleted the more-config-params branch April 25, 2017 21:13
@andrewchilds
Copy link
Contributor Author

Great, thanks @liviucmg! The only thing remaining I think is for you to push the v1.7.0 tag to github and npm publish.

Cheers,
Andrew

@liviucmg
Copy link
Member

Good catch @andrewchilds, all done now. Thanks!

@conanliuhuan
Copy link

thanx

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.

4 participants