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

Merge permalink panel code into permalink component #109

Closed
elemoine opened this issue Jul 10, 2013 · 5 comments
Closed

Merge permalink panel code into permalink component #109

elemoine opened this issue Jul 10, 2013 · 5 comments

Comments

@elemoine
Copy link
Contributor

As discussed during our weekly hangout.

@elemoine
Copy link
Contributor Author

I can work on that on Friday, if no one else takes this on, and if the other works on the permalink panel are done.

@elemoine
Copy link
Contributor Author

I'm actually no longer sure about that. Sorry. I now tend to think that it makes sense to have a "permalink" feature which is just a service that directives like "map", "contextmenu" et "permalinkpanel" use.

What I don't like is the name "permalinkpanel". A component named "share" would make more sense to me.

And talking about names, I think I'd rename "contextmenu" to "contextpopup" or something. What we're displaying on contextmenu events is not a menu really.

@gjn
Copy link
Contributor

gjn commented Jul 12, 2013

+1 on all accounts.

@cedricmoullet
Copy link
Contributor

+1 from me as well

On Fri, Jul 12, 2013 at 9:43 AM, Gilbert Jeiziner
notifications@github.comwrote:

+1 on all accounts.


Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-20862792
.

Twitter: http://twitter.com/cedricmoullet
Linked In: http://www.linkedin.com/in/cedricmoullet

http://twitter.com/cedricmoullet

gjn added a commit that referenced this issue Jul 26, 2013
This was triggere by the discussion at #109
gjn added a commit that referenced this issue Jul 27, 2013
This was triggered by the discussion in #109

Note that this requires a `make clean`
@cedricmoullet
Copy link
Contributor

All has been addressed in the mentionned PR's

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

No branches or pull requests

3 participants