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

Feature/pure components #450

Merged
merged 13 commits into from
Aug 22, 2016
Merged

Feature/pure components #450

merged 13 commits into from
Aug 22, 2016

Conversation

thabti
Copy link
Contributor

@thabti thabti commented Aug 14, 2016

Part of the work to slim down our components and make them state-less whenever possible.

  • Home
  • Surahs
  • Search

@thabti thabti added this to the slim-down components milestone Aug 14, 2016
@ahmedre
Copy link
Contributor

ahmedre commented Aug 14, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 14, 2016

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

import debug from '../../helpers/debug';
const styles = require('./style.scss');

function LastVisit(props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not es6 ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

const LastVisit = ({ lastVisit, surahs }) => {...}

@mmahalwy
Copy link
Contributor

Woot! This is awesome :)

@ahmedre
Copy link
Contributor

ahmedre commented Aug 16, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 20, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@thabti thabti force-pushed the feature/pure-components branch from e05b15f to ae9cb74 Compare August 21, 2016 01:58
@thabti
Copy link
Contributor Author

thabti commented Aug 21, 2016

@mmahalwy @ahmedre this is ready to merged in.

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@thabti thabti changed the title Feature/pure components (Home) Feature/pure components Aug 21, 2016

function mapDispatchToProps(dispatch) {
return {
push: bindActionCreators(push, dispatch)
Copy link
Contributor

Choose a reason for hiding this comment

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

bindActionCreators is actually not needed and is implicit when you return an object with the actions. So { push } does the same as push: bindActionCreators(push, dispatch) no?

Copy link
Contributor

@mmahalwy mmahalwy Aug 21, 2016

Choose a reason for hiding this comment

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

From react-redux docs:

[mapDispatchToProps(dispatch, [ownProps]): dispatchProps](Object or Function): If an object is passed, each function inside it will be assumed to be a Redux action creator. An object with the same function names, but with every action creator wrapped into a dispatch call so they may be invoked directly, will be merged into the component’s props. If a function is passed, it will be given dispatch. It’s up to you to return an object that somehow uses dispatch to bind action creators in your own way. (Tip: you may use the bindActionCreators() helper from Redux.) If you omit it, the default implementation just injects dispatch into your component’s props. If ownProps is specified as a second argument, its value will be the props passed to your component, and mapDispatchToProps will be re-invoked whenever the component receives new props.


it("Should render QuickSurahs component", () => {
let component = shallow(<TopOptions
options={{
Copy link
Contributor

Choose a reason for hiding this comment

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

weird alignment?

@mmahalwy
Copy link
Contributor

@sabeurthabti this is awesome mashallah! Just few comments :)

@thabti
Copy link
Contributor Author

thabti commented Aug 21, 2016

@mmahalwy all fixed.

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@ahmedre
Copy link
Contributor

ahmedre commented Aug 21, 2016

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

@mmahalwy
Copy link
Contributor

@sabeurthabti sweet! Let's merge it :) You have merge capabilities right?

@thabti
Copy link
Contributor Author

thabti commented Aug 22, 2016

@mmahalwy Yeah I do. Merging

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants