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

Tooltipster refactoring #75

Merged
merged 31 commits into from
Oct 31, 2013
Merged

Tooltipster refactoring #75

merged 31 commits into from
Oct 31, 2013

Conversation

louisameline
Copy link
Collaborator

Tooltipster got refactored, hopefully it will be more readable and maintainable. It also got a batch a bug fixes and some new features. The plugin is now more robust and should enable more complex use cases, particularly concerning interactive tooltips.

Changes involve :

  • code cleaning in the structure, choosing clearer variable names, deleted some "var" repetitions. In particular, things have been fixed for tooltips delegated on icons, as that used to be much complicated to know what was going on. Please take a look at the Plugin function to catch it in a glimpse.
  • got rid of unnecessery .data() calls (almost all of them) as there was no reason to bind to the DOM in this manner, preferred instance variables
  • the update method is now immediate, having seperated the checkInterval and the content update process
  • fixed various bindings and unbindings on body
  • a new public method content is available. It acts as a getter when used like .tooltipster('content'), as a setter when used like .tooltipster('content', data). The getter call replaces the val method that has been around for only a few days. The setter call replaces the update method which is now deprecated (but still working for backward compatibility)
  • the provided content and icons can now be jQuery objects with their bindings, the content is now appended and no longer inlined. As a result, the .tooltipster('content') method will return a jQuery object if this is what you provided in the "content" options of the constructor, or via a previous .tooltipster('content', $data) call. Please note that if objects are passed, they will actually be deeply cloned and it is their clone that will be used, unless the new option contentCloning and iconCloning are set to false. Also, if you provide an object as an icon, it will not be applied the iconTheme as it is assumed that you will have styled it by yourself and will want no css property conflicts.
  • fixed a couple bugs for people who would like to change the plugin name (not sure what the point is however)
  • the functionInit behavior got its original behavior back with origin and content parameters (see Added an init callback function in options #63 and changed functionInit behavior #67)
  • fixed queues when delay is 0 (not calling $.delay at all) to let things play synchronously
  • more thorough about the nature of "empty" content : empty strings are valid content and will now provoke the display of an empty tooltip. This means that a title="" attribute results in an empty tooltip being shown. Do not set a title attribute at all if you want to initialize the tooltip without a content (and thus preventing it from showing up). In the same manner, use .tooltipster('content', null) to actually set an empty content via the API. This is also why the default value of the content option is now being set to null rather than an empty string.

Behaviors that have changed

don't worry, almost no code-breaking changes for people who properly use the plugin by its public methods rather than its internals (as they should ;). Just check my last point, but I hope no one was actually using that feature anyway.

  • the tooltip size and position no longer update on content changes that have been made from the outside without using the update method. I believe that kind of "magic" is not good practice anyway, but autoAnimation parameter may be considered at some point, we'll see
  • people who used to bind on the plugin internals like .data() calls on elements may run into trouble, please now use the proper elementIcon, elementTooltip etc methods when you want to build complex interactive tooltips.
  • the onlyOne option now defaults to false to avoid unexpected conflicts with non-autoClosing tooltips, see my commit comment on that
  • .tooltipster('update', data) method is still working but deprecated, please use .tooltipster('content', data) instead
  • the recently added autoclosing of tooltips when they have a select that changes inside them has been removed, as the plugin should not care about what you put inside them. Please use the interactiveAutoClose option and the elementTooltip and hide methods to set up your own bindings and behaviors
  • the plugin does not take script tags off of your content anymore (it was only added a couple days ago anyway). Please understand that it is your job to not let unsafe code get into the plugin content, either by the content option or the title attribute. Script is not the only threat anyway, think of frames or embedded objects for example... Besides, some people may actually want to do this on purpose. So, encode the HTML entities of your content yourself beforehand and you should be safe from XSS, that's a reflex you should have.
  • An exception will now be thrown with "unknown method" message when you make a typo in the method name instead of just failing silently and returning the jQuery object back. That will help you debug hopefully. When you call a tooltipster method, respect the case (like : elementTooltip, not elementtooltip). Remember that Javascript is a case-sensitive language.
  • removed the ability to call the tooltipster method on an uninitialized container element to reach its children, like $('body').tooltipster('hide'). This was unnecessery and slow anyway, please just use some simple $('.mytooltips').tooltipster('hide') bit of code. That was almost not documented anyway, just mentioned in an example in the doc that should be amended

Thank you.

Louis Ameline and others added 30 commits October 27, 2013 16:18
… from the check interval. The tooltip will not react anymore to a content change made without calling the update method, at least for now.
…iginal element, replaced by instance variables. Removed confusing variable names and used the self name convention. Removed useless var repetitions. Removed the awkward getContent function (to be replaced later).
…viously collide with each other or even with the rest of the page before I namespaced
…hat allowed to interact with the tooltips inside a container
…any form, especially an object with bindings etc. It is not tooltipster's job to filter stuff in the content and try to prevent XSS: people should not allow everything in their title attribute in the first place. Besides, the existing getContent function caused issues and not working adequately either
…ip if interactiveAutoClose is true. The plugin should not dictate an arbitrary behavior of some content of the tooltip, users should instead bind their content as they wish to the desired public methods.
…ialized with this value to true currently closes tooltips even if they have been set to interactive and not autoclosable. As the plugin is getting more robust and ready for heavy interactions, this behavior is questionable and should at least be disabled by default. Most people will not see the difference anyway since tooltips are closed quickly by default.
…ipstered class and the regular public method
…e the result can now be jquery objects and not only strings, if that is that's what was provided as content in the first place. Also sorted methods by alphabetical order.
… failing silently and returning 'this'. Will help debug typos.
…ot be called at all so things can happen synchronously
…tip is open, as it is no longer necessary since this interval is no longer in charge of updating the content. Also forgot to set the interval back at 200ms after testing, oops. Done.
…ctually valid content, while only null is the absence of content
…he form of an html object. Now cloning content and icon when it is provided in the form of objects, as a same object can be set as parameter of several instances and should not be shared between them
louisameline added a commit that referenced this pull request Oct 31, 2013
Merge Tooltipster refactoring into dev branch
@louisameline louisameline merged commit a054cd6 into calebjacob:dev Oct 31, 2013
@louisameline louisameline mentioned this pull request Jan 3, 2014
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.

None yet

1 participant