Feature/341 social share #367

thabti · 2016-07-02T20:33:20Z

#341 Social media Share for Surah.

ahmedre · 2016-07-02T20:34:52Z

Deployed to: http://staging.quran.com:32787

ahmedre · 2016-07-02T20:45:41Z

Deployed to: http://staging.quran.com:32788

mmahalwy · 2016-07-02T20:50:08Z

src/components/Share/index.js

+    return (
+      <div className={Style.share}>
+
+        <i onClick={()=> this.onClickPopup(facebookUl, "Facebook")}


facebookUrl misspelled :)

mmahalwy · 2016-07-02T20:53:32Z

dw about this: https://travis-ci.org/quran/quran.com-frontend/builds/141903708#L415

it will get fixed soon!

mahboob-awan · 2016-07-02T21:58:18Z

src/components/Share/style.scss

@@ -0,0 +1,30 @@
+.share {


+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.

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.

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

ahmedre · 2016-07-02T22:05:23Z

Deployed to: http://staging.quran.com:32790

ahmedre · 2016-07-02T22:54:19Z

Deployed to: http://staging.quran.com:32791

ahmedre · 2016-07-02T23:06:12Z

Deployed to: http://staging.quran.com:32792

ahmedre · 2016-07-02T23:07:59Z

Deployed to: http://staging.quran.com:32793

mmahalwy · 2016-07-02T23:23:22Z

src/styles/fonts/_fontawesome.scss

@@ -0,0 +1,2199 @@
+/*!


forgive me, I made a mistake. I thought we already had fontawesome in the code base but we don't Let's go back to what you had earlier with the images. I am so sorry again!

I will suggest to use svg images for icon stuff.

But that is true that technology stack is confusing e.g. i spent time to figure out that .navbar is part of boostrap and we are using bootstrapUI.

So i thought to to add technology stack in "contributing.md" for future reference

e.g.

HTML - Plain HTML + Reactjs Component
CSS - SCSS + Bootstrap
Client-Side JS - Reactjs+Redux
Server-side JS - Express

right?

then i have a question, not sure to ask that here.

which browser stack we are supporting? because usage of HTML/CSS/JS is really dependent on that stack, and will help developers and code-reviewers to evaluate a solution. Thanks

@mahboob-awan agreed. I think slack or skype would be ideal play to discuss these things.

ahmedre · 2016-07-03T01:07:55Z

Deployed to: http://staging.quran.com:32795

thabti · 2016-07-03T01:10:11Z

@mmahalwy reverted back to images.

mmahalwy · 2016-07-03T07:58:44Z

Thank you @sabeurthabti

* HOTFIX reading mode markers (#303) * Default api and files (#306) * Reducing bundle size (#310) * Reducing bundle size * rearranged files * more rearrange and deleted unneeded files * add default segments key (#313) * transitioning to al-quran al-kareem (#314) transitioning to Al-Quran al-Kareem * Fix lint issues * Update Readme.md * Add devDependency Status * Update Readme for new developers * Code splitting assets (#321) * Code splitting * Split the html files into their own and ajax requested * Pass tests * Wrong files * Add segments * Add segments * Autocomplete pointing to v2 suggest * HOTFIX pretty-error dep (#323) * HOTFIX Search to the right route (#324) * No segments for non-segmented reciters (#325) * Adding sentry (#327) * CDN images for surahinfo (#328) * Reduce data dehydration (#329) * Search Action on homepage (#330) * Descriptions for surahs (#331) * Fixed head tags (#332) * Head tags improved and more organized * Fixed the head problem * audioplayer bug fixes, or more bugs (#335) * audioplayer bug fixes, or more bugs * removes some unneeded state things in audioplayer redux, also fixes a bug where event listeners would get bound without properly unbinding multiple times, which caused the player to skip ayahs after using next <-> prev more then once on the same ayah * temporarily disables word focusing capacity * Add sourcemaps to prod * Remove zero padding in urls * (WIP) Issue #248 (#336) Fixes #248 Out of range bug * correct waffle url. (#338) * JS error on surah click (#349) * Add Heroku generated app.json * Ramadan Todo list (#343) * Change * Eslint added (#345) * Eslint added * eslint in travis * ayah require * Fixed * fixed bug that was preventing ayah transition on audioplayer * fix autocomplete thing * Extract connect from the surah container (#347) * Fout fonts (#346) * Fout fonts * wip * wip2 * do not update * pass tests * Sourcemap for sentry (#352) * contribution guideline (#355) * Heroku pipeline trials (#354) * push * push * push * push * push * Heroku push (#356) * Heroku push * public urls * Add zendesk widget (#358) * Fix font artifacts This fixes #365 by merging the 3 fonts from quran/quran.com-images#7, quran/quran.com-images#8, and quran/quran.com-images#9. * Update font for page 237 to fix spacing in 12:17 This fixes the spacing issue in 12:17 by updating the font. Fixes #369. * Feature/341 social share (#367) * Add zendesk widget (#358) * The first commit's message is: * Simplify audioplayer (#360) * Audioplayer refactor * more reshuffling * move logic outside of tracker * fixes #359 * eslint * refactor and works * Added segments * Fix navigating from home to surah * reciter change * on word click working! * segments encryption and lazy decryption * encryption lib * linter is happy * unit tests * env * Fix sentry bug (#373) * Fix sentry bug * no build * #372 colon separated surah/ayah (#374) * #372 colon separated surah/ayah * #372 colon separated surah/ayah * #372 colon separated surah/ayah * Re-output fonts with Apple option enabled This probably doesn't matter for web, but it may matter for apps in the future, plus wanted to keep in sync with the versions of the fonts used by quran/quran.com-images. * Tooltip options for translation and transliteration (#376) * Reading mode single line (#377) * - ayah's have link (discuss, maybe not needed) (#379) - Fix audio player playing text - Remove unneeded padding on mobile - Hide Share on mobile devices (native share is built into the browsers) * Added Mixpanel (#384) * Remove footer links (#385) * Remove footer links * eslint * Fix json-ld (#386)

mahboob-awan · 2016-07-16T23:58:36Z

@mmahalwy are we using paid version of https://symbolset.com/icons/standard?

see Usage

mmahalwy reviewed Jul 2, 2016
View reviewed changes

mahboob-awan reviewed Jul 2, 2016
View reviewed changes

thabti force-pushed the feature/341-social-share branch from c70e607 to 317a814 Compare July 2, 2016 23:04

Add zendesk widget (quran#358)

ccb1ae5

thabti force-pushed the feature/341-social-share branch from 317a814 to 8c05817 Compare July 2, 2016 23:06

mmahalwy reviewed Jul 2, 2016
View reviewed changes

The first commit's message is:

eb8f1dd

thabti force-pushed the feature/341-social-share branch from 8c05817 to eb8f1dd Compare July 3, 2016 01:06

mmahalwy merged commit 1e10b98 into quran:develop Jul 3, 2016

thabti deleted the feature/341-social-share branch July 17, 2016 07:23

harunpehlivan mentioned this pull request Nov 15, 2022

[Snyk] Fix for 1 vulnerabilities harunpehlivan/quran.com-frontend#46

Open

harunpehlivan mentioned this pull request Nov 28, 2022

[Snyk] Fix for 1 vulnerabilities harunpehlivan/quran.com-frontend#47

Open

Feature/341 social share #367

Feature/341 social share #367

Uh oh!

Conversation

thabti commented Jul 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

mmahalwy Jul 2, 2016

Choose a reason for hiding this comment

Uh oh!

mmahalwy commented Jul 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mahboob-awan Jul 2, 2016

Choose a reason for hiding this comment

Uh oh!

thabti Jul 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahboob-awan Jul 3, 2016

Choose a reason for hiding this comment

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

ahmedre commented Jul 2, 2016

Uh oh!

mmahalwy Jul 2, 2016

Choose a reason for hiding this comment

Uh oh!

mahboob-awan Jul 3, 2016

Choose a reason for hiding this comment

Uh oh!

thabti Jul 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahmedre commented Jul 3, 2016

Uh oh!

thabti commented Jul 3, 2016

Uh oh!

mmahalwy commented Jul 3, 2016

Uh oh!

mahboob-awan commented Jul 16, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thabti commented Jul 2, 2016 •

edited

Loading

mmahalwy commented Jul 2, 2016 •

edited

Loading

thabti Jul 2, 2016 •

edited

Loading

thabti Jul 3, 2016 •

edited

Loading