Add collection label next to search results #1068
Conversation
|
Deploy preview for netlify-cms-www ready! Built with commit f08535b |
|
Deploy preview for cms-demo ready! Built with commit f08535b |
There was a problem hiding this comment.
Thanks @solpark!
I'll give a short review each for code and design.
Code
We can simplify things by drilling down to the abstract goal: whenever entries from multiple collections are displayed at once, regardless of why, we should label each entry with it's collection title. The EntryListing component has a renderCardsForMultipleCollections method that handles this specific case, so we can start there without altering components that are higher up in the tree (e.g. passing the isSearchResults flag around).
The renderCardsForMultipleCollections method wraps the EntryCard component directly, so we can simply pass in a label string when we want a label, and don't use the label prop when we don't. This approach should allow us to get away with only modifying two components total.
Design
The approach totally works, but I happen to know that there will eventually be labels or tags or who-knows-what in that right side area of the entry card, so it might not be ideal placement. There's also precedent for placing a UI element's label in the top left, such as in editor fields and editorial workflow cards, both pictured below:
That last example is very similar to the card you're modifying - the cards are from the "Blog" collection, so "BLOG" is indicated in the upper left corner of each. You can probably borrow those styles directly and maybe avoid writing new styles. Also like the last example, you can just use the collection name without prepending "Collection:".
Thanks again for taking the time to contribute! I hope the review was helpful - feel free to ask questions here or in Gitter.
|
@erquhart Thanks for the thorough feedback! I will make the changes ASAP :) |
|
After changes: |
|
@erquhart |
| ] | ||
| }, | ||
| { | ||
| "login": "michaelromani", |
There was a problem hiding this comment.
@solpark Can you confirm that adding "Michael Romani" is intentional? Just wanted to check. 😃
There was a problem hiding this comment.
Yes, worked on this together!
|
@solpark The build should be working now, Netlify was able to get the Yarn issue fixed. |
|
Ah gotcha. Thank you! |
|
Did a bit of editing, borrowed styles from the workflow cards:
|
|
I think this is good to merge - thanks for taking it on @solpark! |
Co-authored-by: Mike Romani 29218846+MichaelRomani@users.noreply.github.com
Co-authored-by: Mike Romani 29218846+MichaelRomani@users.noreply.github.com
Co-authored-by: Mike Romani 29218846+MichaelRomani@users.noreply.github.com
Co-authored-by: Mike Romani 29218846+MichaelRomani@users.noreply.github.com
Co-authored-by: Mike Romani 29218846+MichaelRomani@users.noreply.github.com
- Summary
Closes issue #945
Collection name is indicated on each search result item.
- Test plan
Shows collection name in each search result:
Does not show collection name next to items when not searched:
- Description for the changelog
Add collection label next to search results
- A picture of a cute animal (not mandatory but encouraged)
src: http://i.dailymail.co.uk/i/pix/2012/09/17/article-2204494-150E707A000005DC-700_634x438.jpg