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

Add wagtail hook to create menu items for translations #8606

Merged
merged 5 commits into from
Dec 27, 2024

Conversation

wpears
Copy link
Member

@wpears wpears commented Oct 11, 2024

This PR adds a wagtail hook that leverages the machinery introduced to manage the translation_links to allow easy switching in the admin view between available versions of languages. To wit, see this screenshot:
Screenshot 2024-10-10 at 11 03 19 PM

If a page has no translations, no menu item will be added. If a page has many translations, eg. http://localhost:8000/language/cfpb-in-english/, they'll all be added:

Screenshot 2024-10-10 at 11 12 53 PM

This latter case is a bit unwieldy, but, imo, rare enough that it's worth the general benefit.

This work was spurred on by the old school wagtailadmin_override used in ask-cfpb for the link between english and spanish answer pages:
Screenshot 2024-10-10 at 11 16 18 PM

The idea here being that, if/when we move Ask to the now-standard way of associating translations with each other, it would be cool to retain the functionality of being able to quickly jump to different translations.

Copy link
Member

@willbarton willbarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm fine with this in theory, with the test failure when page isn't in context fixed and maybe with some unit tests of its own.

I do wonder if the page action menu is the right place for this. wagtail-localize puts similar functionality in the header dropdown and page listing's "more" button.


What makes me sad though is that this continues to build on our reinvention of functionality Wagtail already provides, wherein we're not using Locales (but every page has one), we're not using the translation_key (that every page has), and we're not using the built-in routing (because that requires a whole-site translation strategy). So we've got this wheel we've invented. Might as well keep it rolling.

@niqjohnson
Copy link
Member

I love that we're brining this feature out of Ask to other pages.

I'm interested to hear where @csebianlander thinks makes the most sense for the translation links to live in the Wagtail editing interface. I'm also checking with Rebecca to get her take as someone who works in Wagtail a bunch. If we end up wanting to move those links somewhere else, I had a couple of other ideas.

Like @willbarton said, we could move them to the page menu like wagtail-localize does. That would group them with the other page-level, non-publishing commands and also make them available from page listing screens as well as the page editor, which might be handy. Even better would be if we could add them as a flyout menu there, though I don't know off the top of my head if that's something easily done in Wagtail. If it is, it could be a nice way to handle lots of translations without cluttering the top-level menu.

Or we could list all the translations in the "Translation" block on the "Configuration" tab. That's where all of our other translation settings are, so it might be a natural place to look to see what other translations exist. This option feels a little more bespoke to me, though I don't know if that's true code-wise. It also has the drawback of making you go to the page editor and then the configuration tab to get to it, so potentially a few more steps than our other options.

Definitely interested to hear what our content managers think.

@wpears
Copy link
Member Author

wpears commented Oct 11, 2024

What makes me sad though is that this continues to build on our reinvention of functionality Wagtail already provides, wherein we're not using Locales (but every page has one), we're not using the translation_key (that every page has), and we're not using the built-in routing (because that requires a whole-site translation strategy). So we've got this wheel we've invented. Might as well keep it rolling.

ask represents ~ 3/4th of all spanish content, so if we should move in a certain direction with translations of the site, now is the time as we de-specialize ask. Do you have a proposal about how we'd do such a movement to the multiple-page-tree-shared-UUID way of organizing content? Would this just be a massive data migration (since I assume, the current pages each have their own unique translation_key)?

I'd say what I like about this approach here is that it doesn't really make any decisions about how things SHOULD be and just uses what already exists in a way that isn't very messy. If we end up switching to another approach, it would just involve deleting these lines from this one file, no harm no foul.

@willbarton
Copy link
Member

@wpears like I said, I think this is fine, after fixing the test failure when page isn't in context fixed, etc. I do think the page action menu is the wrong place for this, but moving to other places is a simple matter of changing the hook — I agree with @niqjohnson I think @csebianlander should weigh in on where that would be. I have a soft preference for trying to match wagtail-localize where possible.

@csebianlander
Copy link
Contributor

Or we could list all the translations in the "Translation" block on the "Configuration" tab. That's where all of our other translation settings are, so it might be a natural place to look to see what other translations exist. This option feels a little more bespoke to me, though I don't know if that's true code-wise. It also has the drawback of making you go to the page editor and then the configuration tab to get to it, so potentially a few more steps than our other options.

Assuming this is not too bespoke, I like this best, honestly. It's the most intuitive place for me to expect to find these links and doesn't clutter up the save/publish workflow in cases of a full selection of translations. I don't mind having to do some extra UI navigation if it also reinforces the location of this aspect of our workflow in general.

But I also don't hate the implementation as described here. While I share some of @willbarton's reservations re: further extending this bit of custom tooling, the decision to modify site-wide standards for language set up would need to be from content experts as much as technical ones, and is well outside the scope of what's being done here.

@wpears wpears force-pushed the language-in-action-menu branch from b3873ba to c439d32 Compare October 18, 2024 17:42
@wpears
Copy link
Member Author

wpears commented Oct 18, 2024

So it's kinda strange to add the links to the settings panel, since typically how you do this is by adding a "panel" that refers to a field on the model. There is a HelpPanel that allows you to introduce arbitrary HTML but I'm not sure how to access the other page translations when the panels are being defined.

That said, I did try out what this looks like in the page header and I think it works, see below:
Screenshot 2024-10-18 at 10 37 36 AM

@higs4281 higs4281 self-requested a review December 11, 2024 17:43
Copy link
Member

@higs4281 higs4281 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this gets a lot of mileage from one hook, and would be easily backed out or moved as required by future language configurations. The page header menu seems like a friendly place for it.

I vote for fixing up tests and getting it in editors' hands.

@wpears wpears force-pushed the language-in-action-menu branch from c439d32 to 0409d0e Compare December 20, 2024 02:29
Copy link
Contributor

@csebianlander csebianlander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. I'm sensitive to the "not sure this is ultimately where this functionality should live" but given our current situation I'd rather have some way to quickly jump across translations within the editor, and this seems about as graceful a solution within what we have as I could think of.

@mrosemblat
Copy link

LGTM and agreed on approach. Just an FYI to @sonnakim that we are ready to merge this if you're OK with it.

@sonnakim
Copy link
Member

@mrosemblat yes, works for me!

@higs4281 higs4281 added this pull request to the merge queue Dec 27, 2024
Merged via the queue into main with commit b072389 Dec 27, 2024
12 checks passed
@higs4281 higs4281 deleted the language-in-action-menu branch December 27, 2024 16:10
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.

7 participants