Skip to content
This repository was archived by the owner on Jun 28, 2021. It is now read-only.

Feature/341 social share #367

Merged
mmahalwy merged 2 commits into from
Jul 3, 2016
Merged

Feature/341 social share #367

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccb1ae5
Add zendesk widget (#358)
mmahalwy Jun 25, 2016
eb8f1dd
The first commit's message is:
thabti Jul 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add zendesk widget (#358)
  • Loading branch information
@mmahalwy @thabti
mmahalwy authored and thabti committed Jul 2, 2016
commit ccb1ae58211d8437e065c47a19a41779cf6a8108
44 changes: 44 additions & 0 deletions src/components/Share/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React, { Component, PropTypes } from 'react';
const Style = require('./style.scss');

export default class Share extends Component {


static propTypes = {
surah: PropTypes.object.isRequired
};


onClickPopup(url, title) {
window.open(url, title, 'width=670,height=540,scrollbars=no,toolbar=0');
}

render() {

const {surahId, name} = this.props.surah;

const surahUrl = encodeURIComponent(`http://quran.com/${surahId}`);

return (
<div className={Style.share}>

<i
onClick={() => this.onClickPopup(`https://www.facebook.com/sharer/sharer.php?u=${surahUrl}`, 'Facebook')}
className={Style.facebook}
data-metrics-event-name="Share:Facebook"
title="Share on Facebook"
>
</i>

<i
onClick={() => this.onClickPopup(`https://twitter.com/intent/tweet?url=${surahUrl}&text=Surat ${name.simple}`, 'Twitter')}
className={Style.twitter}
data-metrics-event-name="Share:Twitter"
title="Share on Twitter"
>
</i>

</div>
);
}
}
30 changes: 30 additions & 0 deletions src/components/Share/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
.share {
Copy link
Contributor

@mahboob-awan mahboob-awan Jul 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+0
It's great that U did not use any useless css attribute/property but we can improve code by using DRY principle and module specific css selector(that support future extension)

.surah-share{
  &-container{
     position: relative;
     bottom: 2px;
     left: 5px;
  }
 &-icon{
   background-repeat: no-repeat;
   padding-right: 30px;
   background-size: 12px;
   padding-bottom: 10px;
   padding-top: 1px;
 }
 &-facebook{
    @extend .surah-share-icon;
    background-image: url(../../../static/images/FB-grn.png);
    &:hover{
      background-image: url(../../../static/images/FB-beige.png); 
    }
  }
   &-twitter{
    @extend .surah-share-icon;
    background-image: url(../../../static/images/Twitter-grn.png);
    &:hover{
      background-image:  url(../../../static/images/Twitter-beige.png); 
    }
  }
}

Some benefits of above approach:

  • code readability
  • css size is reduced
  • if implement share functionality at different level(e.g. Ayah level), we can define a different select(e.g. ayah-share-facebook) without conflicting with a generic .facebook selector.

Copy link
Contributor Author

@thabti thabti Jul 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

greats tips bro. I have since update the code to introduce font-awesome as @mmahalwy suggested. Please review again and let me know.

The project uses css-module, which doesn't lend it self to nested css.

Copy link
Contributor

@mahboob-awan mahboob-awan Jul 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i did not get " css-module", i am thinking that we are using SCSS that support nesting, right?

http://sass-lang.com/guide#topic-3

position: relative;
bottom: 2px;
left: 5px;
}

.facebook {
background-image: url(../../../static/images/FB-grn.png);
Copy link
Contributor

@mmahalwy mmahalwy Jul 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use font-awesome? http://fontawesome.io/icon/facebook/

Copy link
Contributor Author

@thabti thabti Jul 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of using it, however I thought there would be push against an additional dependency. I'll investigate fontawesome.

Copy link
Contributor Author

@thabti thabti Jul 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, any advice on how to use it within this project? It's quite complicated if I do it from webpack, as it will cause other fonts to stop working.

Copy link
Contributor Author

@thabti thabti Jul 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

background-repeat: no-repeat;
padding-right: 30px;
background-size: 12px;
padding-bottom: 10px;
padding-top: 1px;

&:hover {
background-image: url(../../../static/images/FB-beige.png);
}
}

.twitter {
background-image: url(../../../static/images/Twitter-grn.png);
background-repeat: no-repeat;
padding-right: 30px;
background-size: 21px;
padding-bottom: 10px;

&:hover {
background-image: url(../../../static/images/Twitter-beige.png);
}
}
6 changes: 5 additions & 1 deletion src/containers/Surah/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import Ayah from 'components/Ayah';
import Line from 'components/Line';
import SearchInput from 'components/SearchInput';
import Bismillah from 'components/Bismillah';
import Share from 'components/Share';

// utils
import scroller from 'utils/scroller';
Expand Down Expand Up @@ -407,7 +408,7 @@ export default class Surah extends Component {
}

renderTopOptions() {
const { toggleReadingMode, options } = this.props; // eslint-disable-line no-shadow
const { toggleReadingMode, options, surah } = this.props; // eslint-disable-line no-shadow

return (
<Row>
Expand All @@ -433,6 +434,9 @@ export default class Surah extends Component {
onReadingModeToggle={toggleReadingMode}
/>
</li>
<li>|</li>
<li><Share surah={surah}/></li>

</ul>
</Col>
</Row>
Expand Down
3 changes: 3 additions & 0 deletions src/helpers/Html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ const Html = ({ store, component, assets }) => {
{head.script.toComponent()}
{head.style.toComponent()}

<script dangerouslySetInnerHTML={{__html: `/*<![CDATA[*/window.zEmbed||function(e,t){var n,o,d,i,s,a=[],r=document.createElement("iframe");window.zEmbed=function(){a.push(arguments)},window.zE=window.zE||window.zEmbed,r.src="javascript:false",r.title="",r.role="presentation",(r.frameElement||r).style.cssText="display: none",d=document.getElementsByTagName("script"),d=d[d.length-1],d.parentNode.insertBefore(r,d),i=r.contentWindow,s=i.document;try{o=s}catch(c){n=document.domain,r.src='javascript:var d=document.open();d.domain="'+n+'";void(0);',o=s}o.open()._l=function(){var o=this.createElement("script");n&&(this.domain=n),o.id="js-iframe-async",o.src=e,this.t=+new Date,this.zendeskHost=t,this.zEQueue=a,this.body.appendChild(o)},o.write('<body onload="document._l();">'),o.close()}("https://assets.zendesk.com/embeddable_framework/main.js","quran.zendesk.com");
/*]]>*/`}} />

{Object.keys(assets.styles).map((style, i) => (
<link
href={assets.styles[style]}
Expand Down