Skip to content
This repository has been archived by the owner. It is now read-only.

Extract shared methods to expose common structure #13

Merged
merged 3 commits into from Aug 9, 2017
Merged

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented Jul 31, 2017

Not sure why we had three different ways of getting the mainEdge before. Would this refactor make sense?

@c-w c-w requested a review from erikschlegel Jul 31, 2017
Smarker
Smarker approved these changes Aug 7, 2017
Copy link
Contributor

@Smarker Smarker left a comment

LGTM

@c-w c-w force-pushed the dashboard-extract-shared branch from 63641ce to 081c8a5 Aug 9, 2017
Copy link
Collaborator

@erikschlegel erikschlegel left a comment

LGTM

@@ -145,7 +158,7 @@ export const Dashboard = React.createClass({
<div key={'locations'} className="doughnutChart">
<GraphCard cardHeader={cardHeader}>
<PopularLocationsChart
mainEdge={this.state.categoryValue["name_"+this.state.language]}
mainEdge={this.getMainEdge()}
Copy link
Collaborator

@erikschlegel erikschlegel Aug 9, 2017

I think you're going to run into translation issues here?

Copy link
Contributor Author

@c-w c-w Aug 9, 2017

The method this delegates to already takes care of localization :)

getMainEdge() {
  if (!this.state.categoryValue) {
    return undefined;
  }

  return this.state.categoryValue[`name_${this.state.language}`] || this.state.categoryValue.name;
}

@c-w c-w merged commit 71f8dc9 into master Aug 9, 2017
@c-w c-w deleted the dashboard-extract-shared branch Aug 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants