Skip to content

Fix inconsistent position of translation buttons across admin contexts#914

Open
rypptc wants to merge 2 commits intowagtail:mainfrom
rypptc:fix/translate-button-priority
Open

Fix inconsistent position of translation buttons across admin contexts#914
rypptc wants to merge 2 commits intowagtail:mainfrom
rypptc:fix/translate-button-priority

Conversation

@rypptc
Copy link
Copy Markdown

@rypptc rypptc commented Mar 28, 2026

Fixes #911

Description

The "Translate this page" button appears in different contexts within the Wagtail admin: Page Listing, Page Index, and Page Editor. The "Sync translated pages" button is also affected, as it shares the same priority value within each context.

Its position was inconsistent across these contexts because the hooks defining this behavior used the same priority value, even though each hook operates within a different priority range.

Beyond aesthetics and consistency, the main issue was that in the Page Listing view the button was not visible without scrolling.

As an immediate fix, an internal function has been introduced. This function receives the priority value as a parameter and assigns it through context-specific hook handlers.

AI usage

Used AI assistance to understand the hook priority system and identify appropriate priority values for each context.

@zanzo2003
Copy link
Copy Markdown

Thanks for tackling this, the root cause analysis in #911 is genuinely excellent, probably the best-documented issue in this repo. The fix is on the right track.

What works well

Extracting shared logic into translation_buttons() is exactly the right pattern here. It makes the intent clear and eliminates the duplication that caused the original inconsistency. Registering separate wrappers per hook so each can carry its own priority is the correct architectural decision.

A few things worth addressing before merge

  • The view_name parameter gets silently dropped from page_header_buttons. The register_page_header_buttons hook passes view_name as either "index" or "edit" to distinguish between the Page Index and Page Editor contexts. In practice there's no regression right now since both get treated identically, but dropping the parameter makes the code harder to extend later (e.g. if we ever want different positioning in the editor vs the index). Worth either threading it through or leaving a comment explaining the decision.
  • The chosen priority values (35 for listing, 55 for header) aren't documented anywhere. Given how much care went into mapping out the priority scales in "Translate this page" button position is inconsistent across page menus #911, it would be really helpful to add a short inline comment explaining where each value lands relative to its neighbours, even just a one-liner like # places Translate after Delete (30), before Sort menu order (60). Without that context the numbers look arbitrary, and someone maintaining this a year from now will have to reverse-engineer it.
  • The "Sync translated pages" button is also affected by this change (its priority now varies with the wrapper too), but that's not mentioned in the PR description. Worth a quick note just to make the changelog/review easier to follow.
  • No tests is the main gap. The hook registration and button ordering are straightforward to cover — even a simple assertion that the buttons are registered with the expected priority per hook would prevent this class of regression from sneaking back in.

None of this is blocking in isolation, the core fix is sound. With a comment or two explaining the priority choices and a test, this is ready to go. Happy to help draft the test if that's useful.

…dd tests for button priority per hook context
@rypptc
Copy link
Copy Markdown
Author

rypptc commented Apr 9, 2026

Thanks for the thorough review. I've addressed all four points:

  • Added a comment on view_name explaining it's received because Wagtail passes it, but not used since Page Index and Page Editor share the same priority. If they ever needed different values, view_name would be the way to distinguish them.
  • Added inline comments documenting the priority values and their neighbors in each hook's scale.
  • Updated the PR description to mention that "Sync translated pages" is also affected.
  • Added two tests in TestTranslationButtonPriority verifying the expected priority per hook.

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.

"Translate this page" button position is inconsistent across page menus

2 participants