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

Connect "all volumes" page to Wagtail and Django backend #1055

Merged
merged 7 commits into from
Sep 4, 2024

Conversation

blms
Copy link
Contributor

@blms blms commented Aug 21, 2024

In this PR

  • Integrate data from Wagtail and Django into the new design for the volumes list
    • Simplify the code for handling the returned list of volumes
    • Create a form for handling sort, order, and display (grid vs list)
    • Create a new form widget called CustomDropdownSelect that uses the uk-navbar-dropdown ui components
  • Completely remove the unused all volumes view/template from Django

Questions

  • Do we want author and date added to appear on the grid item for each volume? They're available but commented out since they weren't present in the redesigned page.
  • Is the list view designed?
  • It looks like list view vs grid view is something controlled by the Wagtail content admin/editor, not the end user, in the current configuration. Given that is the case, should we remove this piece from the frontend?

(note that this is reusing a decent amount of #1054, but with Wagtail and not Django view)

@blms blms requested a review from yl5682 August 21, 2024 19:14
@jcmundy
Copy link
Contributor

jcmundy commented Aug 21, 2024

I would think that yes we want the author and date added. The title also needs an ellipsis after a certain number of characters (not important for Sounding Spirit, but Readux.io has some long titles.

If we leave out the list view, the option needs to be removed here:

("List", "List"),

We could also as easily leave in the option with the current styling from
{% if page.layout == 'List' %}

@blms
Copy link
Contributor Author

blms commented Aug 21, 2024

@jcmundy Thanks for the feedback!

To clarify on the last point, my poor wording was intending to ask whether or not this should be removed from Yang's design:

Screenshot 2024-08-21 at 3 57 14 PM

Since it would appear to control something that's currently only controllable from the Wagtail admin, not by a front end user.

That said, we can decide if the list view should exist at all, and if so, whether it should be controlled by admins in Wagtail or by users clicking that menu item.

@jcmundy
Copy link
Contributor

jcmundy commented Aug 21, 2024

Ahhh - yes, that should be removed from the frontend design. :)

@yl5682 yl5682 merged commit e4ee843 into feature/956-single-vol-search Sep 4, 2024
@blms blms deleted the feature/wagtail-all-volumes branch September 9, 2024 14:34
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