Skip to content
This repository has been archived by the owner on Apr 18, 2020. It is now read-only.

added vertical asides (top/bottom) #1

Merged
merged 4 commits into from
Oct 23, 2014

Conversation

seiyria
Copy link
Contributor

@seiyria seiyria commented Oct 22, 2014

I've found your library and I like it a lot, but I also needed an aside (so to speak) but on the bottom. I figured I'd add one on the top as well just in case someone might need that as well. Here's the plunk I've updated the readme with: http://plnkr.co/edit/MoHT8o31AP3xDwF8a9oD?p=preview

@dbtek
Copy link
Owner

dbtek commented Oct 23, 2014

Hi Kyle,

Thanks for the interest. Looks very good to me. One thing I couldn't get is why the use stricts are gone. Can you please explain it?

@dbtek
Copy link
Owner

dbtek commented Oct 23, 2014

I also forked your plunk here and added aside close functionality to the ok/cancel buttons, since there is no backdrop to close.

@seiyria
Copy link
Contributor Author

seiyria commented Oct 23, 2014

Sure. You can hit esc. to close them, but I just wanted to test it w/o backdrop functionality.

I removed the use strict directives because they were in the global scope, and they were trying to tell all of my other code to be strict. I didn't put it back in but if you wanted to, you could put it inside a closure like so:

(function() {
"use strict";
)();

dbtek added a commit that referenced this pull request Oct 23, 2014
added vertical asides (top/bottom)
@dbtek dbtek merged commit c18d0db into dbtek:master Oct 23, 2014
@dbtek
Copy link
Owner

dbtek commented Oct 23, 2014

I always used it on global scope. Of course, there should be a closure for the sake of portability.
Thanks for the info, it's appreciated.

@seiyria
Copy link
Contributor Author

seiyria commented Oct 23, 2014

No problem! I was wondering why my code was breaking - turns out that it was interfering with d3 (since d3 does some things that don't pass in strict mode).

@seiyria
Copy link
Contributor Author

seiyria commented Oct 23, 2014

Hey, I just wanted to let you know that your plunk doesn't work.

@dbtek
Copy link
Owner

dbtek commented Oct 23, 2014

I just noticed it with another browser. It was ok on Safari however it's broken on Chrome (like a very older version).
I will replace it with another plunk here now.

Thanks again 👍

@seiyria
Copy link
Contributor Author

seiyria commented Oct 23, 2014

No problem, thank you for making this!

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

Successfully merging this pull request may close these issues.

None yet

2 participants