-
Notifications
You must be signed in to change notification settings - Fork 40
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
Do not hard-code the "Select from Image Library" string in file_managed_file_browser_open() #6511
Comments
There you go: backdrop/backdrop#4732 ...just a couple of lines changed, and now the title in the modal gets the string from the view name. Rename the view, and you get the new name in the title: |
One big disadvantage of this approach is, that the view name isn't translatable. |
EDIT: actually, regardless of whether you use |
That is only a problem if we see
Having said that, I would like us to add the specific view/display name to the modal title, because I see this issue here as a part of changes that I am suggesting in #2718 and #6512, where the view/display that will be used for the image and file browsers will be configurable. In these cases, including the name of the view/display being used in the title of the modal would be essential in order to provide context of the task being performed. Think "Select from Annual Reports" or "Select from Personal Photos" etc. As an alternative compromise, we could add logic in the code that checks if the specific view/display that is provided by core is used, and if so then it leaves the entire string as At this point though, I would like to point out that this touches on translatable configuration (see #704, #3455, #3522, #4382, #4894 etc.). Things like Views labels should be available for translation regardless. Right? ...in which case, could this be sorted by adding the |
@klonos you need to also call the config with
So even though we're not supposed to translate a variable like |
@herbdool I also had the same concern another time about translating a variable that way, but I was told that it was OK. I cannot recall the exact PR where that has happened, but I was able for example to find the following other instances where we are doing that (not a compete list):
I think that I was pointed to some documentation in d.o actually, which said that it was OK. It was some time ago, so I cannot recall exactly. |
@herbdool looping back to this, I've updated the PR to be using Having said that, I don't see any I would like to move this issue here forward since the simple, 2-line code change addresses the problem this issue was raised for. Allowing certain things like Views names and descriptions, as well as the same for each display etc. to be translated seems like a broader task and out of scope here. We'd need to consider what other things in each view need to be translatable too (such as the the header, footer, no-results text etc.). So lets leave that as a separate follow-up. Perhaps in #4137(?). Anyone care to review/test this? |
This was discussed during the dev meeting today, 33:22 into the recording. Here: https://youtu.be/CTPd63wSpAg?t=2002
|
After a short while and a steep learning curve re file_managed_file_process() - which is the base for all managed file form items, not only image - my conclusion is: whatever we change the However, maybe function file_managed_file_browser_open() tries to cover too many cases, anyway. Currently, as core doesn't use a view for anything besides image browser, and only provides one image browser view, it's OK to just use one function for all and everything. As soon as we try to add something to file fields in core, we'd have to override the AJAX callback function, anyway, to overcome the limitations, I guess. At least, that's what I had to do in my contrib project file_library. 😉 My personal conclusion: the current PR improves things for some cases. But the base problem's still pending. Find some considerations re this render process handling in this otherwise not directly related issue. You have to scroll down a bit. |
Over in #2718, @argiepiano mentioned this forum post where he documents how to mimic the image browser introduced with the Image Library (see #3297) in order to implement a generic, custom file browser/picker (the specific post describes how to do that for pdf files, but can be adapted for any type of file). He mentions the following:
Lets fix that 🙂
The text was updated successfully, but these errors were encountered: