Skip to content

Add dropboxes to UI to choose which type do uid fields point to #123

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

Merged
merged 3 commits into from
Sep 19, 2019

Conversation

paulftw
Copy link
Contributor

@paulftw paulftw commented Sep 18, 2019

This change is Reviewable

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ A review job has been created and sent to the PullRequest network.


Check the status or cancel PullRequest code review here.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

1 Message
The description of this pull request is blank. Adding a high-level summary will help our reviewers provide better feedback.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

Suggest creating a function for getTypeFieldValues. Replace the reduce and remove Object.assign if it is not necessary.


Reviewed with ❤️ by PullRequest

s[0] === "[" && s[s.length - 1] === "]" ? s.slice(1, s.length - 1) : s;
const typeFieldValues = isCreate
? {}
: type.fields.reduce(
Copy link

Choose a reason for hiding this comment

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

This can be moved to another function, eg getTypeFieldValues

${fields
.map(
f =>
`<${f.name}>: ${
Copy link

Choose a reason for hiding this comment

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

This could use selectedTypes[f.name] || f.type

? {}
: type.fields.reduce(
(acc, f) =>
Object.assign(acc, { [f.name]: removeBrackets(f.type) }),
Copy link

Choose a reason for hiding this comment

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

It doesn't seem like Object.assign is needed here. I also think it would be simpler here to write something like:

const typeFieldValues = {};

if (!isCreate) {
  for (const f in type.fields) {
    typeFieldValues[f.name] = removeBrackets(f.type)
  }
}

Rather than using reduce.

@paulftw paulftw merged commit 5813fed into master Sep 19, 2019
@paulftw paulftw deleted the type-to-type branch September 19, 2019 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant