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

fix breadcrumbs again (follow on of #1769) #1770

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

tomkralidis
Copy link
Member

Overview

Mea culpa; forgotten commit of #1769

Related Issue / discussion

#1769

Additional information

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

Updates to public demo

Contributions and licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

Copy link
Contributor

@doublebyte1 doublebyte1 left a comment

Choose a reason for hiding this comment

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

I don't see the need for the change, but maybe I am missing something here? (:

When I click the "schema" or "queryables" breadcrumbs they seem to be already working.

@@ -3,7 +3,7 @@
{% block crumbs %}{{ super() }}
/ <a href="{{ data['collections_path'] }}">{% trans %}Collections{% endtrans %}</a>
/ <a href="{{ data['dataset_path'] }}">{{ data['title'] | truncate( 25 ) }}</a>
/ <a href="./{{ data['id'] }}queryables">{% trans %}Queryables{% endtrans %}</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

@tomkralidis I think you did not touch this line on #1769 and it seems to be working as expected?

@@ -3,7 +3,7 @@
{% block crumbs %}{{ super() }}
/ <a href="{{ data['collections_path'] }}">{% trans %}Collections{% endtrans %}</a>
/ <a href="{{ data['dataset_path'] }}">{{ data['title'] | truncate( 25 ) }}</a>
/ <a href="./{{ data['id'] }}schema">{% trans %}Schema{% endtrans %}</a>
/ <a href="{{ data['dataset_path'] }}/schema">{% trans %}Schema{% endtrans %}</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

@tomkralidis same comment as for queryables.

@tomkralidis
Copy link
Member Author

@doublebyte1 previous to #1769, the breadcrumb href was empty because of missing properties in the data object passed. In some cases, this would cause a 404 when clicked depending on how pygeoapi was deployed (subpath, etc.). This PR (as well as #1769) follows the same HTML rendering pattern as .../items, to safeguard the href.

@doublebyte1 doublebyte1 merged commit 4b28de6 into master Aug 5, 2024
8 checks passed
@tomkralidis tomkralidis deleted the fix-html-collection-breadcrumbs2 branch August 5, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants