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
Fix adding new company through interest form #4162
Conversation
6b23920
to
d180b23
Compare
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.
LGTM 🚀
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.
LGTM
d180b23
to
16a86b4
Compare
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.
Lgtm
16a86b4
to
c4d71c7
Compare
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.
If it works it works 🤷🏼
props.isMulti = true; | ||
} | ||
|
||
if (creatable) { | ||
return ( | ||
<div className={style.field}> | ||
<Creatable | ||
{...props} | ||
isDisabled={disabled} | ||
placeholder={!disabled && placeholder} | ||
instanceId={name} | ||
isMulti | ||
isMulti={props.isMulti} |
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.
Picky, but might as well destruct it from props as well like the others
const companyId = nameOnly ? null : company['value']; | ||
const companyName = nameOnly ? company['label'] : ''; |
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.
const companyId = nameOnly ? null : company['value']; | |
const companyName = nameOnly ? company['label'] : ''; | |
const companyId = nameOnly ? null : company.value; | |
const companyName = nameOnly ? company.label : ''; |
Any specific reason for using squared brackets?
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.
not really, just didn't change it
@@ -447,6 +459,7 @@ const CompanyInterestPage = (props: Props) => { | |||
filter={['companies.company']} | |||
fieldClassName={styles.metaField} | |||
component={SelectInput.AutocompleteField} | |||
creatable |
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.
Why not set the tags
prop here? Isn't that what we want?
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 assumed the tags
-prop was referring to the fact that you can create new tags in tag-selectors. So I figured we should call it something else, as we might want to create other things than tags. Is there some other reason it's called that?
return [false, 'Du må velge en bedrift'] as const; | ||
} else if (value['__isNew__'] || !value.value) { | ||
return [!!value.label, 'Ny bedrift må ha et navn'] as const; | ||
} else { |
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.
Unnecessary else
Description
I think the refactor to react-final-form must have broken the feature where one could add a new company in the company interest form, if it wasn't already in our database.
Result
I made some small front-end only changes to make it send the new company name in the correct format.
Testing
I have sent in a few company interest forms to test. It works well when creating new company interests. It's a teensy bit buggy when editing the name, but do we ever do that?
I think it's fine for a quick fix.