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

bug: add message to browse page when no subcoms #2864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

carlinmack
Copy link
Contributor

@carlinmack carlinmack commented Sep 30, 2024

Children disabled. Before and after:
image

Children enabled (See all visible in after). Before and after:

image

@carlinmack carlinmack force-pushed the list-subcoms-2 branch 2 times, most recently from 95c9faa to fcb0205 Compare September 30, 2024 14:38
/>,
subCommunitiesContainer
);
if (apiEndpoint.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is not clear to me. Is the apiEndpoint unset when there are no sub communities?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, we only include the 'subcommunities' endpoint to the links when the community is allowed to have children which means that the apiEndpoint is otherwise empty

@@ -12,6 +12,7 @@
{% set active_community_header_menu_item= 'browse' %}
{% set subcommunities_endpoint = community["links"]["subcommunities"] %}
{% set community_slug = community["slug"] %}
{% set children_allowed = community["children"]["allow"] %}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already a _show_browse_page here: why is it not working correctly?
Is it in the case you have a direct link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _show_browse_page is for whether the Browse link should appear in the menu. This variable children_allowed is toggling whether we should link to the subcommunities react search kit page. When children aren't allowed this page 404s

@carlinmack carlinmack removed their assignment Oct 1, 2024
<a class="align-self-center rel-ml-1" href="{{url_for('invenio_communities.communities_subcommunities', pid_value=community_slug)}}">
{{ _('See all')}}
</a>
{% endif %}
</div>
<div class="ui divider hidden"></div>
<div id="subcommunities-container"></div>
Copy link
Member

Choose a reason for hiding this comment

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

Move data-api-endpoint config here and change the react application to read this config. Also, the "No subcommunities" message can be computed on the level of the template instead of React.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the changes and tested that it works :)

Copy link
Contributor

@sakshamarora1 sakshamarora1 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@jrcastro2 jrcastro2 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To release 🤖
Development

Successfully merging this pull request may close these issues.

5 participants