Skip to content

Commit

Permalink
[Place Page Revamp] Removes Overview from topics, Adds link to overvi…
Browse files Browse the repository at this point in the history
…ew from Title, Adds world to parent places to highlight. (#4879)

* Adds world to list of places to highlight.
* Makes the place name in the title a link back to the overview page
* Removes the overview from the topics
* Makes the tests check the ordering of the topics.
  • Loading branch information
gmechali authored Jan 23, 2025
1 parent 9e36d52 commit 783332e
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 27 deletions.
4 changes: 2 additions & 2 deletions server/routes/dev_place/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ def place_charts(place_dcid: str):

# Ensure category is valid
place_category = request.args.get("category", place_utils.OVERVIEW_CATEGORY)
if place_category not in place_utils.CATEGORIES:
if place_category not in place_utils.ALLOWED_CATEGORIES:
return error_response(
f"Argument 'category' {place_category} must be one of: {', '.join(place_utils.CATEGORIES)}"
f"Argument 'category' {place_category} must be one of: {', '.join(place_utils.ALLOWED_CATEGORIES)}"
)

# Retrieve available place page charts
Expand Down
12 changes: 4 additions & 8 deletions server/routes/dev_place/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,17 @@
'EurostatNUTS1',
'Country',
'Continent',
'Place', # World!
]

# Place page categories
ORDERED_TOPICS = [
"Economics", "Health", "Equity", "Crime", "Education", "Demographics",
"Housing", "Environment", "Energy"
]
TOPICS = set(ORDERED_TOPICS)
OVERVIEW_CATEGORY = "Overview"
ORDERED_CATEGORIES = [OVERVIEW_CATEGORY] + ORDERED_TOPICS
CATEGORIES = set(ORDERED_CATEGORIES)
ALLOWED_CATEGORIES = {OVERVIEW_CATEGORY}.union(TOPICS)


def get_place_html_link(place_dcid: str, place_name: str) -> str:
Expand Down Expand Up @@ -645,19 +646,14 @@ def get_categories_with_translations(
"""
categories: List[Category] = []

overview_category = Category(
name=OVERVIEW_CATEGORY,
translatedName=get_translated_category_string(OVERVIEW_CATEGORY))
categories.append(overview_category)

categories_set: Set[str] = set()
for page_config_item in chart_config:
category = page_config_item.category
if category in categories_set:
continue
categories_set.add(category)

for category in ORDERED_CATEGORIES:
for category in ORDERED_TOPICS:
if not category in categories_set:
continue
category = Category(name=category,
Expand Down
33 changes: 17 additions & 16 deletions server/webdriver/tests/place_explorer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.support.ui import WebDriverWait

from server.routes.dev_place.utils import ORDERED_CATEGORIES
from server.routes.dev_place.utils import ORDERED_TOPICS
from server.routes.dev_place.utils import TOPICS
from server.webdriver import shared
from server.webdriver.base_dc_webdriver import BaseDcWebdriverTest
from server.webdriver.base_utils import find_elem
Expand Down Expand Up @@ -47,12 +47,11 @@ def test_dev_place_overview_world(self):
self.driver,
path_to_topics=['explore-topics-box'],
classname='item-list-item',
expected_topics=ORDERED_CATEGORIES)
expected_topics=ORDERED_TOPICS)

# And that the categories have data in the overview
block_titles = find_elems(self.driver, value='block-title-text')
self.assertEqual(set([block.text for block in block_titles]),
set(ORDERED_TOPICS))
self.assertEqual(set([block.text for block in block_titles]), TOPICS)

# Assert that every category is expected, and has at least one chart
category_containers = find_elems(self.driver,
Expand All @@ -73,7 +72,7 @@ def test_dev_place_demographics_world(self):
self.driver,
path_to_topics=['explore-topics-box'],
classname='item-list-item',
expected_topics=ORDERED_CATEGORIES)
expected_topics=ORDERED_TOPICS)

# Scroll to a map chart.
# TODO(gmechali): Make a util for scrolling to element.
Expand Down Expand Up @@ -101,7 +100,7 @@ def test_dev_place_overview_california(self):
self.assertIsNotNone(find_elem(self.driver, value='place-info'))
self.assertEqual(
find_elem(self.driver, value='subheader').text,
'State in United States of America, North America')
'State in United States of America, North America, World')

# Asert the related places box exists
self.assertEqual(
Expand All @@ -126,16 +125,15 @@ def test_dev_place_overview_california(self):
self.driver,
path_to_topics=['explore-topics-box'],
classname='item-list-item',
expected_topics=ORDERED_CATEGORIES)
expected_topics=ORDERED_TOPICS)

# And that the categories have data in the overview
topics_in_overview = set([
topics_in_overview = [
"Economics", "Health", "Equity", "Crime", "Education", "Demographics",
"Housing", "Energy"
])
]
block_titles = find_elems(self.driver, value='block-title-text')
self.assertEqual(set([block.text for block in block_titles]),
topics_in_overview)
self.assertEqual([block.text for block in block_titles], topics_in_overview)

# Assert that every category is expected, and has at least one chart
category_containers = find_elems(self.driver,
Expand All @@ -150,15 +148,19 @@ def test_dev_place_overview_africa(self):
"""Ensure experimental dev place page content loads data for a continent."""
self.driver.get(self.url_ + '/place/africa?force_dev_places=true')

# Assert the subheader contains the parent places.
self.assertIsNotNone(find_elem(self.driver, value='place-info'))
self.assertEqual(
find_elem(self.driver, value='subheader').text, 'Continent in World')

# Asert the related places box exists
self.assertEqual(
find_elem(self.driver, value='related-places-callout').text,
'Places in Africa')

categories_for_africa = [
topics_for_africa = [
"Economics", "Health", "Equity", "Demographics", "Environment", "Energy"
]
topics_for_africa = ['Overview'] + categories_for_africa
shared.assert_topics(self,
self.driver,
path_to_topics=['explore-topics-box'],
Expand All @@ -167,14 +169,13 @@ def test_dev_place_overview_africa(self):

# And that the categories have data in the overview
block_titles = find_elems(self.driver, value='block-title-text')
self.assertEqual(set([block.text for block in block_titles]),
set(categories_for_africa))
self.assertEqual([block.text for block in block_titles], topics_for_africa)

# Assert that every category is expected, and has at least one chart
category_containers = find_elems(self.driver,
value='category',
path_to_elem=['charts-container'])
self.assertEqual(len(category_containers), len(categories_for_africa))
self.assertEqual(len(category_containers), len(topics_for_africa))
for category_container in category_containers:
chart_containers = find_elems(category_container, value='chart-container')
self.assertGreater(len(chart_containers), 0)
Expand Down
9 changes: 9 additions & 0 deletions static/css/place/dev_place_page.scss
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ main {
line-height: 36px;
flex-wrap: wrap;

.place-info-link {
color: inherit;
}

.place-info-link:hover {
color: var(--link-color);
text-decoration: underline;
}

.dcid-and-knowledge-graph {
color: var(--gm-3-ref-neutral-neutral-40);
font-size: 12px;
Expand Down
4 changes: 3 additions & 1 deletion static/js/place/dev_place_main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ const PlaceHeader = (props: {
<div className="place-info">
<h1>
<span>
{place.name}
<a className="place-info-link" href={`/place/${place.dcid}`}>
{place.name}
</a>
{category != "Overview" ? ` • ${category}` : ""}{" "}
</span>
<div className="dcid-and-knowledge-graph">
Expand Down

0 comments on commit 783332e

Please sign in to comment.