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

Add label_singular to collection config #1086

Merged
merged 2 commits into from
Feb 19, 2018
Merged

Add label_singular to collection config #1086

merged 2 commits into from
Feb 19, 2018

Conversation

peduarte
Copy link

@peduarte peduarte commented Feb 6, 2018

Hello 👋

This PR adds label_singular to collection config.

Fixes #684

- Summary

This field will be used in the UI in places where a singular label makes more sense. For example:

If you have a "Posts" collection, the "add collection" button in the UI will show as "New Post" rather than "New Posts".

I went with label_singular because I foresee majority of Collection names being in plural. Therefore, I think by default the label should respect the collection name.

However I'm happy to change it to label_plural if needed.

- Test plan

There doesn't seem to be tests for certain fields in the config, such as label, perhaps because these are optional fields. For this reason I have not written any tests.

- Description for the changelog

Add label_singular to collection config

screenshot 2018-02-06 22 22 01

image

@verythorough
Copy link
Contributor

Deploy preview for netlify-cms-www ready!

Built with commit d7ca84a

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

@verythorough
Copy link
Contributor

Deploy preview for cms-demo ready!

Built with commit d7ca84a

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

@verythorough
Copy link
Contributor

verythorough commented Feb 6, 2018

Deploy preview for netlify-cms-www ready!

Built with commit c13a2ce

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

@verythorough
Copy link
Contributor

verythorough commented Feb 6, 2018

Deploy preview for cms-demo ready!

Built with commit c13a2ce

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

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.

This is perfect, thanks @peduarte. Totally agree with the logic behind "label_singular" rather than "plural".

@peduarte
Copy link
Author

peduarte commented Feb 7, 2018

@erquhart that's cool thank you!

I see this PR is now conflicting with master, I can resolve these.

Would you like me to rebase master into this branch too (as per contributing docs)?

@erquhart
Copy link
Contributor

erquhart commented Feb 7, 2018

Just fixed actually - merged a newer PR by mistake earlier and broke a bunch.

@peduarte
Copy link
Author

peduarte commented Feb 7, 2018

@erquhart well thank you! 🙏

@verythorough
Copy link
Contributor

@peduarte Thank you for including the matching docs update!! 😉

label_singular makes sense to me, and matches other CMS configs, like WordPress. We may want to run by @biilmann, since it was his desire to bring automatic pluralization that caused me to propose the label_plural version in the issue. :)

@tech4him1
Copy link
Contributor

@verythorough So I assume it was originally thought of as an override for automatic pluralization, whereas this would be strictly manual?

@peduarte
Copy link
Author

@tech4him1 that's correct. The goal of this implementation is to be strictly manual, as it's been discovered that trying to intelligently pluralize/singularize words doesn't work.

@peduarte
Copy link
Author

peduarte commented Feb 19, 2018

Hey there 👋 is there an "ETA" for this? Thanks! 👍

@verythorough
Copy link
Contributor

I see no blockers, and it looks like @erquhart approved, so I think we're good to go. Thanks for the nudge, and for the contribution! I'm looking forward to making use of this.

@verythorough verythorough merged commit 49894d9 into decaporg:master Feb 19, 2018
@peduarte peduarte deleted the label-singular-config branch February 19, 2018 17:08
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