MosaicGroups / mosaic-groups

A Mosaic groups application
GNU General Public License v3.0
6 stars 2 forks source link

Group join #127

Closed jeffklassen closed 7 years ago

jeffklassen commented 7 years ago

The group join form should be complete

BKreisel commented 7 years ago

A few observations:

Otherwise LGTM. Good work!

jeffklassen commented 7 years ago

@BKreisel What do you mean about the custom couples sign ups? When the group type is 'Couples' it works:

screen shot 2017-04-30 at 9 22 11 am
jeffklassen commented 7 years ago

@BKreisel ah -- I see that if it is 'Married Couples' it does not work. Is that what you are seeing or are you seeing something else?

BKreisel commented 7 years ago

@jeffklassen: Ah yes, I did my testing with a "married couples"group. That appears to be the issue.

BKreisel commented 7 years ago

Addressed with some ES2016 Array.prototype.includes() magic

jeffklassen commented 7 years ago

@BKreisel Man, I really like your solution. We might want to pull that out into its own function. Maybe add this to utils and have a

const isCouplyGroup =(group) ={
   return ['Couples, 'Married couples'].includes(group.audienceType);
} 

just so we are not copying logic around and having to maintain that logic in multiple places.

jeffklassen commented 7 years ago

Oh.. heh.. you did that!!!!