wagtail / wagtail-localize

Translation plugin for Wagtail CMS
https://wagtail-localize.org/
Other
226 stars 86 forks source link

Also add buttons to page header menu #626

Closed th3hamm0r closed 2 years ago

th3hamm0r commented 2 years ago

Currently, if you are on the edit screen of a page and you want to create a translation in another language, or you just want to sync the translation, you have to get to the page listing of the parent, find the target page and use the "more buttons". We've received reports where customers search for those actions and couldn't find them.

It would be nice if the buttons registered with the "register_page_listing_more_buttons"-hook are also registered with the "register_page_header_buttons" (available since 2.16, commit, docs).

I could provide a PR for that. The only challenge is that since 3.0 (commit) the header menu buttons use icons (icon_name), so not providing any looks bad (all other entries already have one), but setting them needs some backwards-compatible code. Otherwise it would fail with wagtail < 3.0, but I think I could handle that.

zerolab commented 2 years ago

Why not have do something like

If WAGTAIL_VERSION >= (3, 0):
  register with icons
if WAGTAIL_VERSION >= (2, 16):
  register without

I will have time to review on Friday

th3hamm0r commented 2 years ago

Ok, I should have been more clear, it was more concerning the call of yield wagtailadmin_widgets.Button(...), but I will wrap it with a function call that basically does that check for each button, just to avoid duplicating too much of the code.

I will try to submit a PR today.

zerolab commented 2 years ago

Fixed by #628