jtrussell/angular-snap.js

Exceptions in Remote Toggle Improvement

Closed this issue · 6 comments

Hi there, thanks for putting this repo together, I am currently trying to get this up and running and hitting some issues but wanted to make a suggestion.

Would it make sense that if an ID is passed into the toggle method, that cant be found, it throw an exception rather than silently failing?

Want to see if that makes sense, for the structure of my spa app seems to be causing issues in the remote service from finding the div.

Once again, thanks again for this great project.

Even in this case it's not actually failing, or at least it isn't designed to silently give up. It's waiting.

It creates a deferred object which will be resolved as soon as a snapper with that ID is registered. This can happen if for example the code which attempts to toggle your snapper is run before angular processes the corresponding snap-content directive. To do otherwise would create some potentially very confusing constraints on your view structure, especially as snap-contents are build up and torn down if they happen to live inside an ng-view, ng-if, or something similar.

I can understand the frustration if you have a typo in your snap ids, I'd suggest creating a constant in your module for each snap ID so you don't have to hard code strings everywhere.

Thanks for the quick reply, as I mention its when it cant be found and in specific to the follow code for the snapRemote:

exports.toggle = function(side, id) {
  id = id || DEFAULT_SNAPPER_ID;
  exports.getSnapper(id).then(function(snapper) {
    if(side === snapper.state().state) {
      exports.close(id);
    } else {
      exports.open(side, id);
    }
  });
};

If the app is calling this on an event caused by user interaction, which I think is the purpose of these handy methods, then not finding that element and throwing an exception would make sense to me as a consumer of the API. For now this issue I am encountering is not a typo, but somehow my structure of elements within a template of a directive is causing these methods to not find a specific drawer, and thus not opening the left panel. I am sure its something I am doing wrong. :)

If this suggestion goes against the grain, no worries.

All the best

Also, there maybe a bug in the recent bower release of your module (1.6.0), I was able to recreate the issue I am seeing in a Plunk. This is your plunk, updated to use the snapRemote, which is working with the original module version you embedded and just an update for calling toggle on the snap-id.

http://plnkr.co/edit/SiW49y

If you take the most recent bower release it will no longer work.

Hmmm... a couple things regarding that plunk:

  1. The snap-id attribute should go on the content element rather than the drawer.
  2. The snap-id value is evaluated as an angular expression not a literal name, e.g.:
<!-- This-->
<snap-content snap-id=" 'leftDrawer' ">
  ...
</span-content>

<!-- NOT this -->
<snap-content snap-id="leftDrawer">
  ...
</span-content>

Note the extra quotes around leftDrawer. This lets you use variables rather than just hard coded literals. I updated that plunk - check it out here.

The reason you were seeing it work with the older version of the library was actually due to a bug in the older version where snapRemote.toggle didn't always honor snap-ids so it was always looking for the default snapper, which is how yours would be registered if snap-id were assigned to the drawer element rather than the content element.

Thanks JT, I just noticed the diff as well when comparing the script, my assumption was that I was doing something wrong, and I was by not completely RTFM.

But I think this comes down to a usability feature that isnt supported by snap,js itself, which is to have various left panels defined for a content tab and open those panels dynamically based on context of the users interaction.

Now I just need to find out why my panel isn't showing anything at all now that it is opening.

Thanks again for the help.

Cool, sorry for the confusion.

It's true the snap.js library itself doesn't provide dedicated tools for context specific drawers - luckily angular is really good at that sort of thing though 😄.

I usually end up using a big ng-switch in my drawer or nesting my snap-content elements as demonstrated here: http://jsbin.com/pakud/8 in reference to jakiestfu/Snap.js#230. Hope that helps!