Skip to content

Extend FilterGroup component with ability to be initially expanded.#14

Merged
anru merged 3 commits intomasterfrom
feature/filters-initialy-expanded
Jun 19, 2017
Merged

Extend FilterGroup component with ability to be initially expanded.#14
anru merged 3 commits intomasterfrom
feature/filters-initialy-expanded

Conversation

@bagratinho
Copy link
Copy Markdown
Contributor

No description provided.


getExpandedState = () => {
return this.props.initialyExpanded ? !this.state.expanded : this.state.expanded;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it looks weird. Why we should rely on initialyExpanded each time we render page ?
initially word has intent to set value only on initialization. like this:

state = {
   expanded: this.props.initialyExpanded,
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@anru I knew that using props values to set initial state is 'anti-pattern', no?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, it's totally ok If you know what are you doing. If you want to set up some initial value for state variable -- well, you are creator of a program.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I was initially thinking of doing it the way you described, than remembered that its not encouraged.

I'll change that.

@anru anru merged commit a3a661e into master Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants