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

Columns misaligned when using Wagtail 2.15 #50

Closed
cnk opened this issue Jan 19, 2022 · 4 comments · Fixed by #51
Closed

Columns misaligned when using Wagtail 2.15 #50

cnk opened this issue Jan 19, 2022 · 4 comments · Fixed by #51

Comments

@cnk
Copy link

cnk commented Jan 19, 2022

The upgrade to Wagtail 2.15 caused the columns in the results table to be misaligned. The header has an extra item at the beginning of the row (for selecting all for bulk actions). And it would appear that allow_navigation=0 to prevent users from navigating away from the page doesn't work since wagtail/wagtail@e93b210

Screenshots

WagtailInventoryIssue

I am honestly not sure what the expected behavior should be for these. I "fixed" the allow_navigation issue in my fork by overriding the _list_explore.html to comment out {% block page_navigation %}

And I elected to make bulk actions work by adding the following code to search.html:

{% block extra_js %}
   {{ block.super }}
   <script>
       window.wagtailConfig.BULK_ACTION_ITEM_TYPE = 'PAGE';
   </script>
   <script defer src="{% versioned_static 'wagtailadmin/js/bulk-actions.js' %}"></script>
{% endblock %}

And then in the results section, I added show_bulk_actions=1 to the _list_explore include and then added the last 2 lines to get the footer which appears when you select one or more of the bulk actions checkboxes.

{% include "wagtailinventory/_list_explore.html" with show_bulk_actions=1 show_parent=1 %}
{% include "wagtailadmin/shared/pagination_nav.html" with items=pages %}
{% trans "Select all pages in listing" as select_all_text %}
{% include 'wagtailadmin/bulk_actions/footer.html' with select_all_obj_text=select_all_text app_label='wagtailcore' model_name='page' objects=pages %}

If you would like to tell me how you would like these 2 items resolved, I could submit a PR - but it may be faster for your team to decide and then put in place whatever fix you have decided on.

@jslay-excella
Copy link
Contributor

I have looked at this today, and played around with some options. Will bring what you have found, as well as what I have found, to the team tomorrow or Monday.

Not sure we need to worry too much about navigation. Is it needed on the inventory page? No. Does it hurt to have it there? Eh... not really either. Also not much of a great way with the implementation from Wagtail to disable it effectively, as you have seen. Which leads me to lean towards just adapting to supporting it being there.

The only other option that I can see off-hand, would be for us to make our own listings template that mimics closely to Wagtails, but this then adds a set of maintenance to this project that I don't think is necessary, just to avoid not having the navigation column.

I think it probably also makes sense to bring in bulk actions, as there may be some use cases for having them.

Will report back when I get feedback from the team.

@jslay-excella
Copy link
Contributor

@cnk We would like your feed back on PR #51

@cnk
Copy link
Author

cnk commented Feb 28, 2022

@jslay-excella Looks good to me. Thanks for adding Wagtail 2.15 compatibility to the inventory report page.

@jslay-excella
Copy link
Contributor

@cnk Sounds good. Waiting on a peer review, then will merge and make a release.

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 a pull request may close this issue.

2 participants