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

Support displayFields in the Relation widget #1303

Merged
merged 4 commits into from
May 1, 2018
Merged

Support displayFields in the Relation widget #1303

merged 4 commits into from
May 1, 2018

Conversation

zurawiki
Copy link
Contributor

- Summary

Some relation objects I have are stored by an id field to guarantee each object has a unique value. My ids are random hashes though and they are not very informative, especially looking through the autocomplete options.

By adding the displayFields option, we can render information of the object that is more useful to the user. In my case I can display the title and author, which helps distinguish posts with the same title.

- Test plan

Manual testing works
The test suite passes using yarn test

- Description for the changelog

The relation widget now supports custom display fields

@zurawiki
Copy link
Contributor Author

By only changing the renderSuggestion callback, I don't have to worry about the onChange prop getting the wrong information. I would appreciate any feedback on this approach.

@verythorough
Copy link
Contributor

verythorough commented Apr 21, 2018

Deploy preview for cms-demo ready!

Built with commit 0d9f19a

https://deploy-preview-1303--cms-demo.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Apr 21, 2018

Deploy preview for netlify-cms-www ready!

Built with commit 0d9f19a

https://deploy-preview-1303--netlify-cms-www.netlify.com

@erquhart
Copy link
Contributor

erquhart commented Apr 21, 2018

@zurawiki man you are on fire, where did you come from? These may not get much activity before Monday, but thank you!

Sent with GitHawk

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

Thanks for this, I love the simplicity of your approach. Can you push a commit that updates the first field in the kitchen sink example to use this? That way we can try it out in the deploy preview.

@@ -90,7 +90,14 @@ class RelationControl extends Component {

renderSuggestion = (suggestion) => {
const { field } = this.props;
const valueField = field.get('valueField');
const valueField = field.get('displayFields').toJS() || field.get('valueField');
Copy link
Contributor

Choose a reason for hiding this comment

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

This would error out if displayFields isn't a list. Instead of converting to JS, just leave the value as is and switch your Array.isArray check to List.isList.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@zurawiki
Copy link
Contributor Author

Thanks for the suggestions @erquhart. I have updated the PR with your feedback.

@erquhart
Copy link
Contributor

Can you add it to the example so we can try it out in the deploy preview? The relevant config field is here: https://github.com/netlify/netlify-cms/blob/1.6.0/example/config.yml#L73-L78

@zurawiki
Copy link
Contributor Author

I had a typo in my previous PR. Should be fixed now and live on the preview site. I guess I can't access the CMS from my GitHub username which makes sense.

@erquhart
Copy link
Contributor

Found an error - try typing "post" into the relation field in the deploy preview, you'll get this error:

Objects are not valid as a React child (found: Wed Apr 25 2018 21:40:39 GMT-0400 (EDT)).

Which means each displayField value is being output as a React element child - in this case a date object from the date field. We'll need to ensure the value being rendered is renderable.

@erquhart erquhart mentioned this pull request Apr 25, 2018
10 tasks
@zurawiki
Copy link
Contributor Author

Ok, thanks for your patience @erquhart.

My patch was to wrap all items in new String() constructors.

I opted to use this approach over .toString() so that null and undefined values would not cause errors. Date object will get translated to proper strings. Array values will render as new String(['a', 'b']) --> 'a,b' which I think is good enough for now.

I am punting on objects, they render as '[object Object]'

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

I think we're there! Just a few documentation change requests.

searchFields: ["name", "twitterHandle"]
valueField: "name"
```
The generated UI input will search the authors collection by name and twitterHandle as the user types. On selection, the author name will be saved for the field.
The generated UI input will search the authors collection by name and twitterHandle as the user types. Each collection will be represented by the author's handle and follower count. On selection, the author name will be saved for the field.
Copy link
Contributor

@erquhart erquhart Apr 26, 2018

Choose a reason for hiding this comment

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

Combine this change into the first sentence, so the first sentence reads:

The generated UI input will search the authors collection by name and twitterHandle, and display each author's handle and follower count.

@@ -22,7 +23,8 @@ The relation widget allows you to reference items from another collection. It pr
name: "author"
widget: "relation"
collection: "authors"
displayFields: ["twitterHandle", "followerCount"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this to the bottom (optional field).

@@ -13,6 +13,7 @@ The relation widget allows you to reference items from another collection. It pr
- **Options:**
- `default`: accepts any widget data type; defaults to an empty string
- `collection`: (**required**) name of the collection being referenced (string)
- `displayFields`: list of one or more names of fields in the referenced collection that will render in the autocomplete menu of the control. The value field is used by default.
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace last sentence with:

Defaults to valueField.

@zurawiki
Copy link
Contributor Author

@erquhart updated

@zurawiki
Copy link
Contributor Author

Do you have any idea on when this PR can be merged in? I'd really like to start benefiting from this feature ASAP.

@erquhart
Copy link
Contributor

@talves @tech4him1 @Benaiah need one more approval here

Sent with GitHawk

@Benaiah Benaiah merged commit cd30b59 into decaporg:master May 1, 2018
@Benaiah
Copy link
Contributor

Benaiah commented May 1, 2018

@zurawiki just merged the changes, thanks for the PR!

@erquhart
Copy link
Contributor

erquhart commented May 5, 2018

@zurawiki getting this released soon.

Sent with GitHawk

brianlmacdonald pushed a commit to brianlmacdonald/netlify-cms that referenced this pull request May 23, 2018
* Support displayFields in the Relation widget

* Fix typo

* Wrap display fields in String constructors

* 📝 Documentation updates
@zurawiki zurawiki deleted the rmz-relation-display-field branch June 24, 2018 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants