wagtail-localize icon indicating copy to clipboard operation
wagtail-localize copied to clipboard

Expected UI issues with Wagtail 3.0 release

Open thibaudcolas opened this issue 3 years ago • 2 comments

The Wagtail 3.0 first release candidate is out. There are large UI changes in this release, for which we have reviewed expected breakage in third-party UI customisations.

This is beyond what we do with our normal breaking changes policy, since the majority of those changes are on parts of Wagtail that haven’t been publicly supported / documented in any way. To make sure this goes smoothly anyway, I’m here to provide an advance notice of what we’re aware of with this specific package 🙂

In the case of wagtail-localize there are three changes we’re expecting to require rework for Wagtail 3.0.

Uppercase text

The majority of the Wagtail admin UI no longer uses uppercase text, to improve readability. The exception is the page status (live, draft, etc.). Suggested actions:

  • Remove all usage of uppercase text in the CMS, except for page status.
  • Aside from CSS, the utility classes u-text-transform-uppercase and label-uppercase no longer exist and shouldn’t be used anymore.

Here are matches in this repository, which will likely need removing:

wagtail/wagtail/wagtail-localize/wagtail_localize/static_src/common/components/Section/index.tsx
10:    text-transform: uppercase;

wagtail/wagtail/wagtail-localize/wagtail_localize/static_src/editor/components/TranslationEditor/segments.tsx
325:    text-transform: uppercase;

Header template reuse

We’ve changed headers across most of the CMS. We are expecting those changes to be cosmetic only and not affect what customisations the header supports, but this is nonetheless worth reviewing manually.

wagtail/wagtail/wagtail-localize/wagtail_localize/locales/templates/wagtaillocales/confirm_delete.html
5:    {% include "wagtailadmin/shared/header.html" with title=view.page_title subtitle=view.get_page_subtitle icon=view.header_icon %}

wagtail/wagtail/wagtail-localize/wagtail_localize/locales/templates/wagtaillocales/edit.html
7:    {% include "wagtailadmin/shared/header.html" with title=view.page_title subtitle=view.get_page_subtitle icon=view.header_icon %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/submit_translation.html
6:    {% include "wagtailadmin/shared/header.html" with title=view.get_title subtitle=view.get_subtitle icon="doc-empty-inverse" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/update_translations.html
7:    {% include "wagtailadmin/shared/header.html" with title=view.get_title subtitle=view.get_subtitle icon="doc-empty-inverse" %}

wagtail/wagtail/wagtail-localize-git/wagtail_localize_git/templates/wagtail_localize_git/dashboard.html
6:    {% include "wagtailadmin/shared/header.html" with title="Pontoon" icon="doc-empty-inverse" %}

Core templates reuse

It’s very hard to assess whether those customisations will suffer from any of our internal changes. This is worth reviewing manually.

wagtail/wagtail/wagtail-localize/wagtail_localize/locales/templates/wagtaillocales/edit.html
1:{% extends "wagtailadmin/base.html" %}
7:    {% include "wagtailadmin/shared/header.html" with title=view.page_title subtitle=view.get_page_subtitle icon=view.header_icon %}
32:                        {% include "wagtailadmin/shared/field_as_li.html" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/locales/templates/wagtaillocales/index.html
1:{% extends "wagtailadmin/generic/index.html" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/locales/templates/wagtaillocales/confirm_delete.html
1:{% extends "wagtailadmin/generic/confirm_delete.html" %}
5:    {% include "wagtailadmin/shared/header.html" with title=view.page_title subtitle=view.get_page_subtitle icon=view.header_icon %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/submit_translation.html
1:{% extends "wagtailadmin/base.html" %}
6:    {% include "wagtailadmin/shared/header.html" with title=view.get_title subtitle=view.get_subtitle icon="doc-empty-inverse" %}
21:                        {% include "wagtailadmin/shared/field_as_li.html" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/edit_translation.html
1:{% extends "wagtailadmin/generic/edit.html" %}
16:    {% include "wagtailadmin/pages/_editor_css.html" %}
20:    {% include "wagtailadmin/pages/_editor_js.html" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/_components.html
9:                {% include "wagtailadmin/shared/field_as_li.html" with li_classes="component-form__fieldname-"|add:field.name %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/update_translations.html
2:{% extends "wagtailadmin/base.html" %}
7:    {% include "wagtailadmin/shared/header.html" with title=view.get_title subtitle=view.get_subtitle icon="doc-empty-inverse" %}
53:                            {% include "wagtailadmin/shared/field_as_li.html" %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/translations_report.html
1:{% extends 'wagtailadmin/reports/base_report.html' %}

wagtail/wagtail/wagtail-localize/wagtail_localize/templates/wagtail_localize/admin/edit_translatable_alias.html
1:{% extends "wagtailadmin/pages/edit.html" %}

I hope this all makes sense. We’ve made a lot of other styling and template changes that are hard to track down, and for which it’s unclear whether any breakage might be expected or not. As you go through the Wagtail 3.0 compatibility work, please let me know if there are other compatibility issues you come across so we can consider those customisations in Wagtail development in the future, and let others know about those breakages.

thibaudcolas avatar Apr 27 '22 10:04 thibaudcolas

FYI - I ran into this trying to edit translated pages on 3.0rc2:

Error during template rendering
In template C:\...\.venv\lib\site-packages\wagtail_localize\templates\wagtail_localize\admin\edit_translation.html, error at line 6

string indices must be integers

1	{% extends "wagtailadmin/generic/edit.html" %}
2	{% load i18n wagtailadmin_tags %}
3	
4	{% block bodyclass %}page-editor{% endblock %}
5	
6	{% block titletag %}{% blocktrans with instance=translation.source.as_instance locale=translation.target_locale %}Translation of {{ instance }} into {{ locale }}{% endblocktrans %}{% endblock %}

The error appears to be in {% blocktrans with instance=translation.source.as_instance locale=translation.target_locale %}

enzedonline avatar Apr 28 '22 13:04 enzedonline

Hey @enzedonline,

we do not yet have official 3.0 support. That is pending. If you have the time, a PR for this would be most welcome

zerolab avatar Apr 28 '22 13:04 zerolab

Should this issue be closed following the 1.2 release which supports Wagtail 3.0? https://github.com/wagtail/wagtail-localize/releases/tag/v1.2

janbaykara avatar Aug 17 '22 12:08 janbaykara

#560 only fixed the header styling. I think we should reporpuse this for the Wagtail 4.0 UI compatibility as 4.0 finishes the UI changes started in 3.0

zerolab avatar Aug 17 '22 13:08 zerolab

Sounds good. Happy to volunteer on this as we are launching a project soon that would ideally lean on Wagtail 4.0 + wagtail-localize. I'm currently looking in to potential issues.

janbaykara avatar Aug 17 '22 15:08 janbaykara

That would be great.

The biggest thing will be header markup and overall styling. At the moment we still retain the teal colours whereas the 4.0 UI is much lighter

zerolab avatar Aug 17 '22 16:08 zerolab

Just making notes here of things to look into:

  • Need to use the get_revision_model method in the migrations
  • CSS variables need updating (--w-)

janbaykara avatar Aug 18 '22 09:08 janbaykara

Not to question fundamentals too much but is there a particular reason why the whole of the translation editor is implemented in React?

Wondering if it wouldn't be easier to maintain alignment with the Wagtail admin if it this package leant on the same header templates. For example, the new page editor's header has metadata stuff going on etc that would be nice not to lose.

I'm just getting stuck into this though, there might be an obvious reason.

janbaykara avatar Aug 18 '22 15:08 janbaykara

Not to question fundamentals too much but is there a particular reason why the translation editor is implemented in React?

This was one of the first questions I had when I started working on localize. The short answer is there was supposed to be a lot more functionality that lent itself nicely with React. However, the scope and priorities shifted since.

I would be all up for dropping the React requirement as it would also make fixing things like #508 or #364 faster too, but I don't have enough dedicated time to make it happen.

zerolab avatar Aug 18 '22 15:08 zerolab

OK, so I think dropping React makes sense given the context. The impending introduction of StimulusJS into Wagtail means decent interactivity could be layered on top at a later date if required.

For the sake of speed and compatibility with 4.0, I'll submit a PR that brings in just enough of the header template that's required, but keeps the majority of the editor in React.

janbaykara avatar Aug 18 '22 17:08 janbaykara

this was fixed in #602 and the follow up #613 It is far from feeling native, but it is much closer to the Wagtail 4.0 interface than before

zerolab avatar Sep 23 '22 10:09 zerolab