-
-
Notifications
You must be signed in to change notification settings - Fork 67
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
1141 map select options #1149
1141 map select options #1149
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, this looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments!
I'm looking into the failing build pipeline. |
onChange={() => updateTypesFilter(type.typeId)} | ||
/> | ||
} | ||
label={type.typeName} | ||
/> | ||
))} | ||
<FormControlLabel | ||
key={'all'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you'll need to wrap the key value in curly braces if it's just a string.
Hey Edwin, just left one more tiny comment and everything looks good to go! I haven't had the time to look into the build pipeline yet. You're welcome to look into it if you'd like, otherwise I'll try to resolve it and merge in your PR this weekend. |
@joshuayhwu, I just merged some updates into this branch so the build checks pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Thanks for the speedy review Joshua! |
Fixes #1141
dev
branchAny questions? See the getting started guide