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

Adopt core shared header for ModelAdmin headers #8907

Closed
lb- opened this issue Jul 26, 2022 · 4 comments · Fixed by #9993
Closed

Adopt core shared header for ModelAdmin headers #8907

lb- opened this issue Jul 26, 2022 · 4 comments · Fixed by #9993

Comments

@lb-
Copy link
Member

lb- commented Jul 26, 2022

Is your proposal related to a problem?

ModelAdmin header is one of the last major header usage within the admin that is not using the using the core (non-slim) shared header template.

Describe the solution you'd like

  • Using either a template that extends the shared header OR the template include, ensure that the wagtail/contrib/modeladmin/templates/modeladmin/index.html view uses the shared header.
  • This is so that shared styles will all work the same and do not need to be maintained across multiple files.
  • Must preserve existing ability to override the modeladmin header and relevant templates to add custom content.

Describe alternatives you've considered

  • Leave as is, ok to maintain these in isolation for the most part.

Additional context

Notes for potential implementation approach

  • Most likely need to use template inheritance for this, create a new wagtail/contrib/modeladmin/templates/modeladmin/includes/header.html that extends wagtail/admin/templates/wagtailadmin/shared/header.html
  • Add suitable blocks to the shared/header if needed, rough outline below; (note: as this is an extends, we will probably need a with added in the wagtail/contrib/modeladmin/templates/modeladmin/index.html to pass down the right variables.
    • {% block h1 %} inside <div class="col">
    • {% if view.header_icon %} will need to be replace with that icon being passed into the include
    • view.get_page_title will need to be passed into the include
    • self.get_page_subtitle will need to be passed into the include
    • {% include 'modeladmin/includes/result_count.html' %} will need to be passed into the include as description (with a release note change that this now appears outside the h1 tag (which makes sense)
    • {% if search_url %} will need to be wrapped in a block {% block search %}
    • {% block header_extra %} should probably be set up as a fragment and passed into extra_actions on the include (this is a small breaking change, this block will no longer render outside .right column but this should be ok to
  • Clean up - we should drop the support of block right_column_classname and ensure the styles still work, remove any now unused styles, add release note about this no longer being supported.
@lb- lb- changed the title UX Unification - adopt shared header for modeladmin & snippets headers Adopt shared header for modeladmin & snippets headers Sep 10, 2022
@lb- lb- changed the title Adopt shared header for modeladmin & snippets headers Adopt shared header for Headers Sep 19, 2022
@lb- lb- changed the title Adopt shared header for Headers Adopt shared header for ModelAdmin headers Sep 19, 2022
@lb- lb- changed the title Adopt shared header for ModelAdmin headers Adopt core shared header for ModelAdmin headers Sep 19, 2022
@salty-ivy
Copy link
Member

salty-ivy commented Jan 30, 2023

@lb- working on this one first, then followed by #9227 as you suggested.

@salty-ivy
Copy link
Member

@lb- PR is Ready.

@lb-
Copy link
Member Author

lb- commented Jan 31, 2023

Thanks. I'll flag it for review.

lb- pushed a commit to salty-ivy/wagtail that referenced this issue Feb 23, 2023
@lb-
Copy link
Member Author

lb- commented Feb 23, 2023

Will need to raise a new issue after this - the clean up step above and probably should try to align the right/left class names with BEM naming if possible.

lb- pushed a commit to salty-ivy/wagtail that referenced this issue Feb 23, 2023
@lb- lb- closed this as completed in #9993 Feb 24, 2023
lb- pushed a commit that referenced this issue Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants