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

OnListFind method in NgRest/Base/Plugin fix #762

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Antikon
Copy link
Contributor

@Antikon Antikon commented Aug 17, 2024

What are you changing/introducing

Replaced language from default to active.

What is the reason for changing/introducing

The current state results in text always being displayed in the default language rather than the requested one.

QA

Q A
Is bugfix? yes
New feature? no
Breaks BC? no
Tests pass? yes

@nadar
Copy link
Member

nadar commented Aug 18, 2024

Thanks, @Antikon . This is a bit of a BC (backward compatibility) break, so we'll need to think it through. This change will affect how i18n values are displayed in the grid view, based on the user's interface language.

I believe the general admin interface language can change for users, but the data integrity and input should remain consistent based on the developer who created the CRUD.

For example, if you have two languages, but one is the primary language while the other is mostly empty, using the admin interface language that is mostly empty would result in empty grids.

I'll discuss this internally. What was your reasoning behind this change? What's the motivation?

@Antikon
Copy link
Contributor Author

Antikon commented Aug 18, 2024

Hmm, I thought it was just a mistake. I was wrong.

Up to now CRUD multilingual data is always displayed in the default language specified in the admin_lang table. It is assumed that the data is most complete in this language. And indeed, if there is no data for the admin language, an empty field will be displayed.

However, I wanted to ensure that switching the interface language also affected the display of data. If the default language is English, and I want to have an admin in German and I have all the data for this, then why not show it?

As a result, I have a proposal to either display data in the default language, if it is there, and it is not in the requested language. Or we need to create some way to explicitly indicate which language is required.

@nadar
Copy link
Member

nadar commented Aug 21, 2024

I think the biggest problem with this change, is that there are many UI languages the user can choose from (in the user settings) and maybe you do not provided those languages in the CRUD - so they always have empty lists, no matter what.

The only thing i could accept about this PR, is that there is luya\admin\Module settings $property which is false by default, for example: $useUserAdminUILanguageAsDefaultInListView = false

@nadar nadar marked this pull request as draft August 25, 2024 18:27
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.

2 participants