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.

Sign up for GitHub

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

Jump to bottom

Allow experience domains creation in task creation #4119

Merged
merged 6 commits into from
Jun 13, 2019
Merged

Allow experience domains creation in task creation #4119

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b4d5d1c
made it possible to add exp domains in task creation
MichaelBuessemeyer Jun 1, 2019
cc4e969
Update CHANGELOG.md
MichaelBuessemeyer Jun 3, 2019
0abb4e0
removed mode property of SelectExperienceDomain
MichaelBuessemeyer Jun 3, 2019
3dc897b
Merge branch 'master' into allow-exp-domain-creation
MichaelBuessemeyer Jun 13, 2019
f440378
options of experience selection are no longer modified directly
MichaelBuessemeyer Jun 13, 2019
fd0954c
Merge branch 'allow-exp-domain-creation' of github.com:scalableminds/…
MichaelBuessemeyer Jun 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion frontend/javascripts/admin/task/task_create_form_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import SelectExperienceDomain from "components/select_experience_domain";
import messages from "messages";

const FormItem = Form.Item;
const Option = Select.Option;
const { Option } = Select;
const RadioGroup = Radio.Group;

type Props = {
Expand Down Expand Up @@ -255,6 +255,7 @@ class TaskCreateFormView extends React.PureComponent<Props, State> {
placeholder="Select an Experience Domain"
notFoundContent={messages["task.domain_does_not_exist"]}
width={100}
allowCreation
/>,
)}
</FormItem>
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/admin/user/experience_modal_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class ExperienceModalView extends React.PureComponent<Props, State> {
) : null}
<SelectExperienceDomain
disabled={false}
mode="tags"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mode property is no longer needed here. It served the purpose to be able to select the entered text/experience domain although it is not included in the suggestions. As I implemented this behaviour manually, this is no longer required. Please note that the approach "changing mode to 'tags' in task_create_form_view" does change the behaviour of the select and thus this is not an option for the task creation view.

allowCreation
placeholder="New Experience Domain"
value={[]}
width={50}
Expand Down
50 changes: 40 additions & 10 deletions frontend/javascripts/components/select_experience_domain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as React from "react";
import type { ExperienceDomainList } from "admin/api_flow_types";
import { getExistingExperienceDomains } from "admin/admin_rest_api";

const Option = Select.Option;
const { Option } = Select;

type Props = {
value?: string | Array<string>,
Expand All @@ -17,20 +17,24 @@ type Props = {
mode?: string,
onSelect?: string => void,
onChange?: () => void,
allowCreation: boolean,
alreadyUsedDomains: ExperienceDomainList,
};

type State = {
domains: ExperienceDomainList,
currentlyEnteredDomain: string,
};

class SelectExperienceDomain extends React.PureComponent<Props, State> {
static defaultProps = {
alreadyUsedDomains: [],
allowCreation: false,
};

state = {
domains: [],
currentlyEnteredDomain: "",
};

componentDidMount() {
Expand All @@ -45,21 +49,47 @@ class SelectExperienceDomain extends React.PureComponent<Props, State> {
return this.state.domains.filter(domain => !this.props.alreadyUsedDomains.includes(domain));
}

onSearch = (domain: string) => {
this.setState({ currentlyEnteredDomain: domain });
};

render() {
const {
mode,
value,
notFoundContent,
width,
disabled,
placeholder,
onSelect,
onChange,
allowCreation,
} = this.props;
const { currentlyEnteredDomain } = this.state;
const options = this.getUnusedDomains();
if (
allowCreation &&
!options.includes(currentlyEnteredDomain) &&
currentlyEnteredDomain.trim() !== ""
) {
options.push(currentlyEnteredDomain);
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit hesitant about the modification of options here. Currently it's not a problem as state.domains is filtered in getUnusedDomains and returns a new array, but this could easily change in the future. This could then result in a hard to find state modification error.
Maybe change options to a let and then do options = [...options, currentlyEnteredDomain];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for paying attention to this 👍

}

return (
<Select
showSearch
mode={this.props.mode}
value={this.props.value}
mode={mode}
value={value}
optionFilterProp="children"
notFoundContent={this.props.notFoundContent}
style={{ width: `${this.props.width}%` }}
disabled={this.props.disabled}
placeholder={this.props.placeholder}
onSelect={this.props.onSelect}
onChange={this.props.onChange}
notFoundContent={notFoundContent}
style={{ width: `${width}%` }}
disabled={disabled}
placeholder={placeholder}
onSelect={onSelect}
onChange={onChange}
onSearch={this.onSearch}
>
{this.getUnusedDomains().map(domain => (
{options.map(domain => (
<Option key={domain}>{domain}</Option>
))}
</Select>
Expand Down