-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add the complete file path to the manage file form. #4175
Comments
Yes, according to that description text. But I think "Description" is an even worse label for it! I like the description text on the Maybe we should call this |
@laryn where did you find that UI? I just added a file field to a node and it's not there... edit: Oh, I found an "enable description field" checkbox... but I didn't check it because I didn't think the file label was a description.... even though we just covered this! |
I suppose using what is already used elsewhere makes sense here, and then opening a separate issue to suggest that we change that descriptor throughout, if it's no good? Separately, this thread is related: |
Yep, good idea. Will use Description here now, and I'll add the description text I like too :) edit: here's the other issue for deciding what to name it #4178 |
Thanks @jenlampton 👍 ...I wouldn't mind holding off until we've made a decision in #4178 and got this all in one go ...I think we'll reach to a decision soon 😉 |
...I've left comments in the PR. I love this ❤️ ...just minor tweaks implementation-wise; is all. |
Hey @jenlampton ...I wanted to git this a go, but the PR sandbox doesn't seem to work, and there seem to be conflicts (after merging of #4178 I guess). Can you please resolve the conflict, and make sure that the sandbox works? ...if not, then perhaps try closing the PR and reopening it 🙂 |
There is one test failure right now that looks like it is probably legit, I'm not sure I understand what is going on there. Can anyone help me with that? |
@stpaultim I've tested the new PR with a PDF file, and it looks good to me: The vertical tab looks like in your screenshot, the File URL is clickable and opens in a new small window. |
The changed code looks great, thanks @herbdool for coming in and fixing the use of There's a legit failing test across all versions of PHP:
Looks like it just needs to be updated for the new markup created by this PR. |
I fixed |
Thanks @herbdool! I merged backdrop/backdrop#4619 into 1.x and this is the first issue in the new 1.27.x branch to be fixed. Thanks @jenlampton, @stpaultim, @laryn, @olafgrabienski, and @klonos! |
When I'm managing my files, I really want to know the file name (not the human-readable label we already show in the form, but the actual name of the document on disk). I also sometimes wonder where that file is stored, because I'd like to test it directly in my browser. We don't reveal any of this information to the user, and it seems both are a valuable part of managing files.
I'd like to propose the following improvements to the Manage File form:
Change the label of the "File name" element to indicate this is a labelScreenshot:
I went with "File link text" in the screenshot (and first PR) but am open to recommendations on what to call the human-friendly name for a fileI went withDescription
as the label for that field, since that is what we are calling the same thing in other interfaces. See #4178 for better text to use there.PR: backdrop/backdrop#2960
Translation updates:
The text was updated successfully, but these errors were encountered: