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

Enable "explore in tool" on dev place pages #4785

Merged
merged 3 commits into from
Dec 11, 2024

Conversation

juliawu
Copy link
Contributor

@juliawu juliawu commented Dec 11, 2024

Adds the "explore in [timeline/map] tool" to chart footers in the dev place pages.

Also sorts fields alphabetically for codacy code quality.

Example:
Screenshot 2024-12-11 at 10 00 48 AM

@juliawu juliawu requested review from dwnoble and gmechali December 11, 2024 18:02
Copy link
Contributor

@gmechali gmechali left a comment

Choose a reason for hiding this comment

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

Honestly I haven't done it in some of my PRs on the revamped place page, but should we be including this kind of stuff in our webdriver test?

"""Ensure experimental dev place page content loads"""

Should we validate the explore more is present now to ensure there won't be a regression?

Otherwise LGTM

@dwnoble
Copy link
Contributor

dwnoble commented Dec 11, 2024

Honestly I haven't done it in some of my PRs on the revamped place page, but should we be including this kind of stuff in our webdriver test?

"""Ensure experimental dev place page content loads"""

Should we validate the explore more is present now to ensure there won't be a regression?
Otherwise LGTM

Agreed on adding this check

@juliawu
Copy link
Contributor Author

juliawu commented Dec 11, 2024

Honestly I haven't done it in some of my PRs on the revamped place page, but should we be including this kind of stuff in our webdriver test?

"""Ensure experimental dev place page content loads"""

Should we validate the explore more is present now to ensure there won't be a regression?
Otherwise LGTM

This is a great idea. Added a check to the webdriver test.

@juliawu juliawu merged commit dfe8688 into datacommonsorg:master Dec 11, 2024
8 checks passed
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.

3 participants