Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

This adds pinterest as an option on social icons, and adds per-slide sharing links. #834

Merged
merged 4 commits into from

2 participants

@chee
Collaborator

The code might be mental. if it is, i am sorry.

RFC

app/assets/javascripts/lib/components/gallery.js
((5 lines not shown))
};
Gallery.prototype._updateImageInfo = function() {
var slideDetails = this.slider.$currentSlide.find(".js-slide-details"),
caption = slideDetails.find(".caption").text(),
poi = slideDetails.find(".poi").html(),
- breadcrumb = slideDetails.find(".breadcrumb").html();
+ breadcrumb = slideDetails.find(".breadcrumb").html(),

Can these four selectors use js- prefixed classes?

@chee Collaborator
chee added a note

yes they certainly should, shouldn't they? yes i'll fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ianfeather Ianfeather commented on the diff
app/assets/javascripts/lib/components/gallery.js
((14 lines not shown))
this.galleryTitle.text(caption);
this.galleryPoi.html(poi);
this.galleryBreadcrumb.html(breadcrumb);
+ this.gallerySocial.html(social);

Can you add a spec for this? Should be pretty clear/easy to continue the one I wrote.

@chee Collaborator
chee added a note

sure thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ianfeather Ianfeather commented on the diff
app/views/components/_social_buttons.html.haml
@@ -1,5 +1,8 @@
-- encoded_url = CGI.escape(properties[:url])
-- tweet = properties[:tweet] || encoded_url
+:ruby
+ show = properties[:show] || %w(facebook twitter google-plus stumbleupon)
+ show = show - (properties[:hide] || [])

This is nice. It would probably have been enough just to do pinterest, but cool - nice and flexible

@chee Collaborator
chee added a note

Yeah, I suppose I don't know if we'll ever need it. But it's nice that if there is a page where, for example, the stumbleupon icon makes more sense displayed somewhere other than the social bar we can just pass hide: ['stumbleupon']

i'm glad i found this code. i thought i'd chee'd it, i wrote it a few weeks ago in rockton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ianfeather

Looks good otherwise

@chee
Collaborator

Cool. Thanks, Ian. I'll fix those things up, :housekiller:.

Related: lonelyplanet/waldorf#1361

@chee
Collaborator

Yay.

@chee chee merged commit 5d21303 into from
@chee chee deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
9 app/assets/javascripts/lib/components/gallery.js
@@ -44,17 +44,20 @@ define([
this.galleryTitle = this.$gallery.find(".js-gallery-title");
this.galleryPoi = this.$gallery.find(".js-gallery-poi");
this.galleryBreadcrumb = this.$gallery.find(".js-gallery-breadcrumb");
+ this.gallerySocial = this.$gallery.find(".js-gallery-social");
};
Gallery.prototype._updateImageInfo = function() {
var slideDetails = this.slider.$currentSlide.find(".js-slide-details"),
- caption = slideDetails.find(".caption").text(),
- poi = slideDetails.find(".poi").html(),
- breadcrumb = slideDetails.find(".breadcrumb").html();
+ caption = slideDetails.find(".js-slide-caption").text(),
+ poi = slideDetails.find(".js-slide-poi").html(),
+ breadcrumb = slideDetails.find(".js-slide-breadcrumb").html(),
+ social = slideDetails.find(".js-slide-social").html();
this.galleryTitle.text(caption);
this.galleryPoi.html(poi);
this.galleryBreadcrumb.html(breadcrumb);
+ this.gallerySocial.html(social);

Can you add a spec for this? Should be pretty clear/easy to continue the one I wrote.

@chee Collaborator
chee added a note

sure thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
};
Gallery.prototype._updateSlug = function(partial) {
View
39 app/views/components/_social_buttons.html.haml
@@ -1,5 +1,8 @@
-- encoded_url = CGI.escape(properties[:url])
-- tweet = properties[:tweet] || encoded_url
+:ruby
+ show = properties[:show] || %w(facebook twitter google-plus stumbleupon)
+ show = show - (properties[:hide] || [])

This is nice. It would probably have been enough just to do pinterest, but cool - nice and flexible

@chee Collaborator
chee added a note

Yeah, I suppose I don't know if we'll ever need it. But it's nice that if there is a page where, for example, the stumbleupon icon makes more sense displayed somewhere other than the social bar we can just pass hide: ['stumbleupon']

i'm glad i found this code. i thought i'd chee'd it, i wrote it a few weeks ago in rockton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ encoded_url = CGI.escape(properties[:url])
+ tweet = properties[:tweet] || encoded_url
.nav--inline
- if properties[:show_label?]
@@ -7,19 +10,27 @@
Share this
.social
+ - if show.include? 'facebook'
+ %a.social__item.social__item--facebook{ href: "https://www.facebook.com/sharer/sharer.php?u=#{encoded_url}", data: {lpa_category: 'share', lpa_action: 'facebook', lpa_label: "#{properties[:url]}"} }
+ %span.accessibility Share on Facebook
+ %span.social__icon.icon--facebook.icon--white
- %a.social__item.social__item--facebook{ href: "https://www.facebook.com/sharer/sharer.php?u=#{encoded_url}", data: {lpa_category: "share", lpa_action: "facebook", lpa_label: "#{properties[:url]}"} }
- %span.accessibility Share on Facebook
- %span.social__icon.icon--facebook.icon--white
+ - if show.include? 'twitter'
+ %a.social__item.social__item--twitter{ href: "https://twitter.com/intent/tweet?text=#{tweet}", data: {lpa_category: 'share', lpa_action: 'twitter', lpa_label: '#{properties[:url]}'} }
+ %span.accessibility Share on Twitter
+ %span.social__icon.icon--twitter.icon--white
- %a.social__item.social__item--twitter{ href: "https://twitter.com/intent/tweet?text=#{tweet}", data: {lpa_category: "share", lpa_action: "twitter", lpa_label: "#{properties[:url]}"} }
- %span.accessibility Share on Twitter
- %span.social__icon.icon--twitter.icon--white
+ - if show.include? 'google-plus'
+ %a.social__item.social__item--google-plus{ href: "https://plus.google.com/share?url=#{encoded_url}", data: {lpa_category: 'share', lpa_action: 'google-plus', lpa_label: "#{properties[:url]}"}}
+ %span.accessibility Share on Google+
+ %span.social__icon.icon--google-plus.icon--white
- %a.social__item.social__item--google-plus{ href: "https://plus.google.com/share?url=#{encoded_url}", data: {lpa_category: "share", lpa_action: "google-plus", lpa_label: "#{properties[:url]}"}}
- %span.accessibility Share on Google+
- %span.social__icon.icon--google-plus.icon--white
+ - if show.include? 'stumbleupon'
+ %a.social__item.social__item--stumbleupon{ href: "http://www.stumbleupon.com/submit?url=#{encoded_url}", target: "_blank", data: {lpa_category: "share", lpa_action: "stumbleupon", lpa_label: "#{properties[:url]}"}}
+ %span.accessibility Share on Stumbleupon
+ %span.social__icon.icon--stumbleupon.icon--white
- %a.social__item.social__item--stumbleupon{ href: "http://www.stumbleupon.com/submit?url=#{encoded_url}", target: "_blank", data: {lpa_category: "share", lpa_action: "stumbleupon", lpa_label: "#{properties[:url]}"}}
- %span.accessibility Share on Stumbleupon
- %span.social__icon.icon--stumbleupon.icon--white
+ - if show.include? 'pinterest'
+ %a.social__item.social__item--pinterest{ href: "http://pinterest.com/pin/create/button/?url=#{URI.encode(request.original_url)}&media=#{URI.encode(properties[:url])}&description=#{properties[:description]}", target: "blank" }
+ %span.accessibility Pin this image
+ %span.social__icon.icon--pinterest.icon--white
View
8 spec/javascripts/fixtures/gallery.html
@@ -6,15 +6,17 @@
<div class="js-gallery-title">A sample title</div>
<div class="js-gallery-poi">A sample POI</div>
<div class="js-gallery-breadcrumb">A sample breadcrumb</div>
+ <div class="js-gallery-social">A sample social</div>
</div>
<div class="js-slide">Dummy slide because slider needs > 2</div>
<div class="js-slide fixture-incoming-slide">
<div class="js-slide-details">
- <div class="caption">A new caption</div>
- <div class="poi">A new poi</div>
- <div class="breadcrumb">A new breadcrumb</div>
+ <div class="js-slide-caption">A new caption</div>
+ <div class="js-slide-poi">A new poi</div>
+ <div class="js-slide-breadcrumb">A new breadcrumb</div>
+ <div class="js-slide-social">A new social paradigm</div>
</div>
</div>
View
7 spec/javascripts/lib/components/gallery_spec.js
@@ -52,9 +52,10 @@ require([ "jquery", "public/assets/javascripts/lib/components/gallery.js" ], fun
});
it("updates the gallery with the new slide details", function() {
- expect(gallery.galleryTitle.text()).toBe($(".fixture-incoming-slide").find(".caption").text());
- expect(gallery.galleryPoi.html()).toBe($(".fixture-incoming-slide").find(".poi").text());
- expect(gallery.galleryBreadcrumb.html()).toBe($(".fixture-incoming-slide").find(".breadcrumb").text());
+ expect(gallery.galleryTitle.text()).toBe($(".fixture-incoming-slide").find(".js-slide-caption").text());
+ expect(gallery.galleryPoi.html()).toBe($(".fixture-incoming-slide").find(".js-slide-poi").text());
+ expect(gallery.galleryBreadcrumb.html()).toBe($(".fixture-incoming-slide").find(".js-slide-breadcrumb").text());
+ expect(gallery.gallerySocial.html()).toBe($(".fixture-incoming-slide").find(".js-slide-social").text());
});
});
Something went wrong with that request. Please try again.