Improve "place in" box on dev place pages #4774
Conversation
| const { place, childPlaces } = props; | ||
| if (!childPlaces || childPlaces.length === 0) { | ||
| return null; | ||
| } | ||
| const NUM_PLACES = 15; |
There was a problem hiding this comment.
curious - how did we choose 15?
There was a problem hiding this comment.
Admittedly a bit arbitrarily -- this looks good to my eyes for now. My thought is to use NUM_PLACES = 15 to unblock experimental release before the code freeze, and we can decide on a better solution after.
There was a problem hiding this comment.
After thinking about this more, i think a better experience might be to make this responsive and to show more items when the page is larger, and fewer for smaller screen sizes. Wdyt @miss-o-soup ?
There was a problem hiding this comment.
Sounds good, I'll discuss with @miss-o-soup and add responsive behavior in a follow-up PR, I don't think this needs to block release.
static/js/place/dev_place_main.tsx
Outdated
| </a> | ||
| </div> | ||
| ))} | ||
| {childPlaces |
There was a problem hiding this comment.
I might argue this is starting to add a lot of logic within the render block.
could we maybe store the slice in a different variable called trimmedChildPlaces and then just toggle between trimmedChildPlaces and allChildPlaces?
There was a problem hiding this comment.
Good call out. Stored the slice in a different variable and now just toggle between the lists which looks much cleaner now.
|
Just from the screenshots -- looks like we are returning Yugoslavia. We should be filtering "deprecated" countries.. |
Agreed-this is already on our task list |
static/js/place/dev_place_main.tsx
Outdated
| {showToggle && ( | ||
| <div className="show-more-toggle" onClick={toggleShowMore}> | ||
| <span className="show-more-toggle-text"> | ||
| {isCollapsed ? "Show more" : "Show less"} |
There was a problem hiding this comment.
Can you add the count for show more? e.g., "Show 25 more"
There was a problem hiding this comment.
Great idea, done! Also updated screenshots in the PR description.
|
@gmechali @dwnoble @miss-o-soup as per offline discussion, added the removal of the place search bar to this PR. Please take another look! |
1) Fix long list of child places on continent place pages
On continent place pages, the list of child places will contain places that are more than one level down in containment (e.g, showing municipalities, not just countries). This PR adds a child place type entry for continents to the child place fetch logic.
Before:
After:
2) Add a "show more" toggle when the list of child places is long
Sometimes the list of child places is very long, which overwhelms the page. This PR also adds a "show more" toggle when the number of child places is greater than 15.
Long lists are truncated initially
Show full list after clicking "show more"
Short lists don't show a toggle
3) Replace place search with link to knowledge graph on current place pages
In preparation for the changes made in PR #4740, this PR also removes the place search bar to avoid showing users double search bars, and instead shows the DCID and a link to the knowledge graph entry, fixing a longstanding feature regression.
Before:
After:
