NGSTACK-829 add translation indication below object name #10

ljacmi · 2024-02-07T17:13:15Z

No description provided.

emodric · 2024-02-08T08:56:08Z

bundle/Resources/views/themes/ngadmin/content/location_view.html.twig

                <a href="{{ url }}">{{ content_type.name }}</a>
+                {% set locale_conversion_map = ibexa.configResolver.getParameter('locale.conversion_map', 'ngsite') %}
+                {% for language in content.versionInfo.languages %}
+                    &nbsp {{ locale_conversion_map[language.languageCode]|split('_')|last|nglayouts_country_flag|raw }}


&nbsp (should be   btw) is an error here I guess? Why not just separate languages with commas?

Also, don't use nglayouts_country_flag, this package cannot depend on Netgen Layouts. If needed, implement a custom Twig function to fetch the language name with LanguageService from Ibexa.

Also, don't use nglayouts_country_flag, this package cannot depend on Netgen Layouts. If needed, implement a custom Twig function to fetch the language name with LanguageService from Ibexa.

Nije greška, komentar na povezanom tasku na fini glasi:
"pliz ne zaboravi dodat razmak između content tip linka i ovih zastavica. Na Fini je nabijeno"

Može se promijenit, al meni se i ovo čini ok

@emodric

Also, don't use nglayouts_country_flag, this package cannot depend on Netgen Layouts. If needed, implement a custom Twig function to fetch the language name with LanguageService from Ibexa.

@iherak suggested we put this in media-site instead of here. Your thoughts?

That is project code, this is open source. This open source package cannot depend on Netgen Layouts. And since there will be no flags, there should be a comma separating the language names.

The alternative is to reimplement here the function to generate flags.

@iherak suggested we put this in media-site instead of here. Your thoughts?

Not ideal. Only new websites will then benefit from that unless we proactively implement it on older sites. Upgradablilty is also a problem. The template is not completely trivial, so any change here later will need to be propagated manually.

@iherak suggested we put this in media-site instead of here. Your thoughts?

Not ideal. Only new websites will then benefit from that unless we proactively implement it on older sites. Upgradablilty is also a problem. The template is not completely trivial, so any change here later will need to be propagated manually.

@emodric Do you have a suggestion then?
EDIT: Reiplement the function?

My suggestion still stands. Just list the comma separated language names. Implement a Twig function to fetch the language by its language code and use the language object to fetch the name e.g. ngadmin_language('cro-HR').name

@emodric Is this ok now?

…of languages

emodric · 2024-02-08T11:34:31Z

bundle/Templating/Twig/BetterIbexaAdminUIExtension.php

+    public function getFilters()
+    {
+        return [
+            new TwigFunction(


You're using a Twig function under filters. This should be moved to getFunctions.

emodric · 2024-02-08T11:35:02Z

bundle/Templating/Twig/BetterIbexaAdminUIExtension.php

+    {
+        return [
+            new TwigFunction(
+                'show_language_name',


This should be ngadmin_language so we can fetch the object instead of the name, which allows some more flexibility.

emodric · 2024-02-08T11:35:21Z

bundle/Templating/Twig/BetterIbexaAdminUIRuntime.php

    }
+
+    /**
+     * @throws NotFoundException


The method does not throw, so this is redundant.

emodric · 2024-02-08T11:35:45Z

bundle/Resources/views/themes/ngadmin/content/location_view.html.twig

+                {% for language in content.versionInfo.languages %}
+                    {% set array = array|merge([language.languageCode|show_language_name]) %}
+                {% endfor %}
+                &nbsp {{array|join(', ') }}


&nbsp is not needed now.

It is IMO.

Without:

With:

Why not add the list of languages below the content type name?

Besides, &nbsp should really be   (with a semicolon).

Idk, flags were in the same row. It can be either way I guess: This is how it was done originally so I didn't change it.

emodric · 2024-02-08T12:04:59Z

Wait wait wait wait... It seems that content.versionInfo.languages is already a list of language objects, meaning you don't need the Twig function/filter after all 🤦‍♂️. You can just use language.name directly in the iteration! 🤣

ljacmi · 2024-02-08T13:01:53Z

Wait wait wait wait... It seems that content.versionInfo.languages is already a list of language objects, meaning you don't need the Twig function/filter after all 🤦‍♂️. You can just use language.name directly in the iteration! 🤣

Done

emodric · 2024-02-08T14:19:25Z

bundle/Resources/views/themes/ngadmin/content/location_view.html.twig

                </a>
                <a href="{{ url }}">{{ content_type.name }}</a>
+
+                {% set array = [] %}


One final nitpick, this variable should be named something like languages or language_names

emodric · 2024-02-08T14:52:38Z

Thanks @ljacmi ! I will leave this to @pspanja to merge and release.

iherak · 2024-02-08T14:54:08Z

bundle/Resources/views/themes/ngadmin/content/location_view.html.twig

                </a>
                <a href="{{ url }}">{{ content_type.name }}</a>
+
+                {% set language_names = [] %}


Why is the logic within span if we are rendering this outside of the span? :)

pspanja

It seems I missed this. Thanks @ljacmi!

Miljenko Muha added 2 commits February 7, 2024 18:09

NGSTACK-829 added language indication below content name

22a1fbb

NGSTACK-829 some empty liner added

f8641a1

ljacmi requested a review from iherak February 7, 2024 17:13

emodric reviewed Feb 8, 2024

View reviewed changes

NGSTACK-829 removed dependency to nglayouts, flags replaced by names …

83445be

…of languages

emodric reviewed Feb 8, 2024

View reviewed changes

Miljenko Muha added 2 commits February 8, 2024 12:50

NGSTACK-829 twig function made filter

7f8a18c

NGSTACK-829 filter returns language instead of just its name

31d4ff9

Miljenko Muha added 5 commits February 8, 2024 13:05

NGSTACK-829 names moved to new row

cd3f4c3

NGSTACK-829 whitespace added

fb93b89

NGSTACK-829 removed twig filter as it is not necessary

86af949

NGSTACK-829 removed unnecessary use statements

d70fc30

NGSTACK-829 removed unnecessary service

9553307

emodric reviewed Feb 8, 2024

View reviewed changes

NGSTACK-822 array variable renamed

453d421

emodric approved these changes Feb 8, 2024

View reviewed changes

emodric requested a review from pspanja February 8, 2024 14:52

iherak approved these changes Feb 8, 2024

View reviewed changes

iherak reviewed Feb 8, 2024

View reviewed changes

iherak self-requested a review February 8, 2024 14:54

NGSTACK-829 lines moved outside span

6b32083

iherak approved these changes Feb 9, 2024

View reviewed changes

pspanja approved these changes May 3, 2024

View reviewed changes

pspanja merged commit 8a42221 into master May 3, 2024

pspanja deleted the NGSTACK-829-add-translation-indication-below-object-name branch May 3, 2024 07:39

NGSTACK-829 add translation indication below object name #10

NGSTACK-829 add translation indication below object name #10

Uh oh!

Conversation

ljacmi commented Feb 7, 2024

Uh oh!

emodric Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljacmi Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljacmi Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljacmi Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emodric commented Feb 8, 2024

Uh oh!

ljacmi commented Feb 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emodric commented Feb 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pspanja left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

emodric Feb 8, 2024 •

edited

Loading

ljacmi Feb 8, 2024 •

edited

Loading

ljacmi Feb 8, 2024 •

edited

Loading

ljacmi Feb 8, 2024 •

edited

Loading