Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exquisite Corpse #8

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Exquisite Corpse #8

wants to merge 15 commits into from

Conversation

LemonyDesign
Copy link

In development…

}

addFavourites(corpseitem) {
this.setState({
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than using a callback here, it would be better to use

const favourites = this.state.favourites.concat(corpseitem);
this.setState({
  favourites: favourites
});
window.localStorage.setItem('favourites', favourites);

}

removeFromFavourites(corpseitem) {
const filteredFavourites = this.state.favourites.filter(currentFavourite => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice


const favouriteclasses = cx('workshop__favourite', {
'active': this.state.active,
'': !this.state.active
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line is needed as it will insert nothing in both cases

function Generate({receiveType, typeresult1, typeresult2, favourites, addFavourites, removeFromFavourites}) {

function handleChange1(event) {
receiveType(event.target.value, 'type1');
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if type1 and type2 could have more descriptive names as they seem a bit vague


const corpsesclasses = cx('generate__corpses menu--settings', {
'active': notEmpty && sameLength,
'': !(notEmpty && sameLength)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line needed. It does not add any classes

@dmitrigrabov
Copy link
Contributor

Great work

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

Successfully merging this pull request may close these issues.

2 participants