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

Fix adding new company through interest form #4162

Merged
merged 1 commit into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/components/Form/SelectInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type Props = {
value: any;
disabled?: boolean;
options?: Record<string, any>[];
creatable?: boolean;
isMulti?: boolean;
};

export const selectStyles = {
Expand Down Expand Up @@ -100,17 +102,23 @@ function SelectInput({
options = [],
disabled = false,
placeholder,
creatable,
...props
}: Props) {
if (props.tags) {
creatable = true;
props.isMulti = true;
}

if (creatable) {
return (
<div className={style.field}>
<Creatable
{...props}
isDisabled={disabled}
placeholder={!disabled && placeholder}
instanceId={name}
isMulti
isMulti={props.isMulti}
Comment on lines +110 to +121
Copy link
Member

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

onBlur={() => onBlur(value)}
value={value}
isValidNewOption={isValidNewOption}
Expand Down
19 changes: 16 additions & 3 deletions app/routes/companyInterest/components/CompanyInterestPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,18 @@ const requiredIfEventType = (eventType: string) =>
return event && event.checked;
});

const validateCompany = (value) => {
if (!value) {
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 {
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary else

return [typeof value?.value !== 'number', 'Ugyldig bedrift'] as const;
}
};

const validate = createValidator({
company: [required()],
company: [validateCompany],
contactPerson: [required()],
mail: [required(), isEmail()],
phone: [required()],
Expand All @@ -348,8 +358,10 @@ const CompanyInterestPage = (props: Props) => {

const onSubmit = (data) => {
const { company } = data;
const companyId = company['value'] ? Number(company['value']) : null;
const companyName = companyId === null ? company['label'] : '';
const nameOnly = company['__isNew__'] || !company.value;
const companyId = nameOnly ? null : company['value'];
const companyName = nameOnly ? company['label'] : '';
Comment on lines +362 to +363
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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?

Copy link
Member Author

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


const [range_start, range_end] = data.participantRange
? PARTICIPANT_RANGE_MAP[data.participantRange]
: [null, null];
Expand Down Expand Up @@ -447,6 +459,7 @@ const CompanyInterestPage = (props: Props) => {
filter={['companies.company']}
fieldClassName={styles.metaField}
component={SelectInput.AutocompleteField}
creatable
Copy link
Member

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?

Copy link
Member Author

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?

required
/>
<Field
Expand Down