Use correct language in panels #1717
Conversation
|
@amureki did you happen to test loading a previous request via the history panel to confirm that the language is correct then? |
Honestly, I didn't, as I was not aware of such specifics. But I can try to add a test for it tomorrow, if I will find a good reference. |
|
Sorry, I didn't mean a unit test, but rather testing with the application as a whole. So when you discovered the bug that this PR fixes, did you also use the history panel? |
|
@tim-schilling ah, I see what you mean, sorry Tim. I just quickly checked, history panel had also this bug (table titles were in website language, not toolbar language), and it is fixed by this patch. 👍 |
|
Perfect, thank you! |
|
@tim-schilling oh wait. It does switch back to the I will try to dig into it more tomorrow to see what is causing this issue and what was missed... |
|
@amureki this is a more complicated change than expected. I'm going to push a change shortly that should address more of the issues. |
|
@matthiask can you review the changes in this PR when you're available? |
PR #1703 introduced a bug where SQLPanel (and probably others) is rendered in `LANGUAGE_CODE` language, while it should use `TOOLBAR_LANGUAGE` if provided. This PR fixes panel's content language.
for more information, see https://pre-commit.ci
This reviews the third-party panel documentation to reference these two decorators and explain them. They aren't necessary but should be used.
There was a problem hiding this comment.
I'm not completely sure if it wouldn't be better to send the toolbar's language as an additional query parameter to the individual views, because now you could still get a toolbar with mixed languages by changing the language preference after the initial page load (by going into the browser settings)
But that may also be a new requirement (or enhancement). FWIW I think the change is fine. I'm not that happy with the aliased imports (dt_settings) but that's the way it's done in the toolbar source so that's fine with me.
So we're on the same page, what would be your preference? |
|
@tim-schilling I opened a new issue for this at #1720 , let's continue the discussion about aliases there. |
|
@amureki Do you have an opinion regarding the following comment?
|
|
@matthiask I am generally happy with @tim-schilling solution for this problem. In my opinion, it solves it for the majority of the usage cases. I'd be happy to report any further issues I'll experience with the languages, but we definitely can move on with merging this PR. |
Description
Greetings fellows,
PR #1703 introduced a bug where SQLPanel (and probably others) is rendered in
LANGUAGE_CODElanguage, while it should useTOOLBAR_LANGUAGEif provided.This PR fixes panel's content language.
I am not sure what else could be missed, I am also open to other implementations.
Best,
Rust
Checklist:
docs/changes.rst.