Adds sharing functions to widget

Adds sharing functions to widget

-Share links
-Links open in new window/tab

https://code.launchpad.net/~jcsackett/juju-gui/make-em-work/+merge/171535

(do not edit description out of merge proposal)

[j.c.sackett, 2013-06-26 12:44:27]

Please take a look.

[rharding, 2013-06-26 13:28:48]

LGTM but I really think we should be just sending a simplified link straight to
the charm vs the current url as it might have all kinds of search, inspector,
etc bits to it that are completely out of context for other users.

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
app/subapps/browser/views/charm.js:652: link: window.location.href,
Should this be the current location or should it just be a hard coded link to
the charm itself. So jujucharms.com/charm/id that the View rendering the widget
can use? (e.g. should the link include search and such when it's kind of out of
context.)

[j.c.sackett, 2013-06-26 13:43:25]

So, inspector bits aren't in the URL anymore. Per the url design discussions
(last month, I think?) url state only contains browser state, not
environment/inspector state.

I take your point about search though; querystring isn't part of what's shared,
but the '/search/' bit is. How do you feel about me landing another branch to
deal with that, since I'm leery of agreeing anything with URL generation is
"simple" given past experience, and I'd rather people be able to see this sooner
than later.

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
app/subapps/browser/views/charm.js:652: link: window.location.href,
On 2013/06/26 13:28:48, rharding wrote:
> Should this be the current location or should it just be a hard coded link to
> the charm itself. So jujucharms.com/charm/id that the View rendering the
widget
> can use? (e.g. should the link include search and such when it's kind of out
of
> context.)

See my reply to your main comment in your review.

[bac, 2013-06-26 15:40:09]

LGTM modulo Rick's concerns.  For me I'd be happy to see it land sooner and then
modified with a follow-up branch.

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/10609043/diff/1/app/subapps/browser/views/char...
app/subapps/browser/views/charm.js:19: 
why two blank lines?

geez, don't you hate having to defend blank lines?  sorry.

https://codereview.appspot.com/10609043/diff/1/app/templates/sharing-widget.h...
File app/templates/sharing-widget.handlebars (right):

https://codereview.appspot.com/10609043/diff/1/app/templates/sharing-widget.h...
app/templates/sharing-widget.handlebars:3: <a
href="http://twitter.com/home?status={{ twitter_text }}" title="Twitter this">
"Twitter" as a verb?  I thought it was "Tweet".  But, I'm sure this came from
the UX guys.

Oh, and now I see it is just the title.  NM.

https://codereview.appspot.com/10609043/diff/1/app/templates/sharing-widget.h...
app/templates/sharing-widget.handlebars:8: <a
href="https://plus.google.com/share?url={{ link }}" title="G+ this">
Hmm, even if it is just a title I think "Post on G+" makes more sense.  Again,
defer to UX.

https://codereview.appspot.com/10609043/diff/1/test/test_sharing_widget.js
File test/test_sharing_widget.js (right):

https://codereview.appspot.com/10609043/diff/1/test/test_sharing_widget.js#ne...
test/test_sharing_widget.js:77: 
Maybe a test showing the text of the message is escaped.

[j.c.sackett, 2013-06-26 19:06:07]

*** Submitted:

Adds sharing functions to widget

-Share links
-Links open in new window/tab

R=rharding, bac
CC=
https://codereview.appspot.com/10609043