wagtail / wagtail-localize

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

Wagtail 4.0 compat #602

Closed janbaykara closed 2 years ago

janbaykara commented 2 years ago

This PR addresses incompatibilities between wagtail-localize and Wagtail 4.0rc2, following discussion in #558:

Changes

Approach

The approach I've taken is to use copy the old React codebase into a static_src subdirectory, and conditionally render it based on WAGTAIL_VERSION < 4.0. This is because styling, CSS variables and the relationship between the React codebase and Django templates have changed between Wagtail versions in a complex way.

Not in scope

Screenshots

Screenshot 2022-08-18 at 22 50 13

Previous UI:

Screenshot 2022-08-18 at 22 55 32
janbaykara commented 2 years ago

@zerolab I've not fully tested this yet but wanted to put out a first draft. Would appreciate any pointers so we can bring this home.

janbaykara commented 2 years ago

Right now the template header seems to lack some of the right hand icons/actions that normal pages have, unsure how to activate those.

zerolab commented 2 years ago

Hey @janbaykara nice start. My main grip about the approach is that we still support Wagtail 2.15 as an LTS, so the changes then become incompatible with it.

My preferred approach would be to have legacy styles and compontents used for Wagtail < 4.0, then all the new ones.

janbaykara commented 2 years ago

@zerolab perhaps we can run a version check and then load up the right version of the template / component for the wagtail version?

zerolab commented 2 years ago

@janbaykara we take this approach across the codebase, so 👍🏼

janbaykara commented 2 years ago

I think this is ready for review @zerolab @kaedroho

zerolab commented 2 years ago

Hey @janbaykara, thank you so much for this.

I am looking at this now. I see there are lots of whitespace changes in the tsx files (specifically, going from 4 spaces to 2 spaces). Our .editorconfig configuration is set up to use 4 spaces. Can you please tweaks the files accordingly?

janbaykara commented 2 years ago

I am looking at this now. I see there are lots of whitespace changes in the tsx files (specifically, going from 4 spaces to 2 spaces). Our .editorconfig configuration is set up to use 4 spaces. Can you please tweaks the files accordingly?

I'm not 100% but I hope running yarn format will have fixed this.

I've also run pre-commit run --all-files to fix up the python formatting issues.

Have fixed up the other review points.

janbaykara commented 2 years ago

Hm, seems we need to do a WAGTAIL_VERSION check before importing things and messing with the view code too. Makes sense, will action. https://github.com/wagtail/wagtail-localize/runs/8110770886?check_suite_focus=true

zerolab commented 2 years ago

Hey @janbaykara thank you very much for this. I've made the necessary tweaks to get it over the line. See #613 Closing this

janbaykara commented 2 years ago

No worries! Glad you've been able to make progress.