Improve download interactions #6509
Conversation
With help from Palid. This does two major things: 1. Makes the tile part of a file body clickable to trigger a download. 2. Refactors a lot of the recyclable code out of the DownloadActionButton in favour of a utility. It's not a perfect refactoring, but it sets the stage for future work in the area (if needed). The refactoring still has a heavy reliance on being supplied an iframe, but simplifies the DownloadActionButton and a hair of the MFileBody download code. In future, we'd probably want to make the iframe completely managed by the FileDownloader rather than have it only sometimes manage a hidden iframe.
Turns out the tooltip component doesn't copy over class names.
turt2live
added
Sponsored
T-Task
turt2live
added
the
X-Release-Blocker
| blob: blob, | ||
| download: name, | ||
| auto: autoDownload, | ||
| }, '*'); |
There was a problem hiding this comment.
Should we use location.origin over *. It looks like that iframe will and should always be loaded on the same origin as the parent page
There was a problem hiding this comment.
possibly, though postMessage is known to be fiddly with origins so would advocate that we do it in a different PR
|
Ideally the filename truncation should happen in the middle of the filename or a few characters before the filename extension, cause otherwise it's not going to be obvious for everyone that the three last chars are part of the extension name. Example of filename truncation on macOS. On the 2nd screenshot the expanded file name tooltip is obscuring the file size, which could be prevented by using a greater vertical offset. On the 3rd screenshot the file size is also getting truncated, accidentally obscuring that data. In general, I think this component is in need of a much better design overall, so that the file extension and file size are not affected by the filename length, etc. But that's out of the scope of this PR. |
|
@janogarcia would it be okay to add the file size to the tooltip to cover all 3 issues for the short term? |
|
@turt2live Let's do that for the short term, yes. |
|
@janogarcia done; ptal :) |
Fixes element-hq/element-web#18238
Fixes element-hq/element-web#18239
With help from @Palid
See commits for individual descriptions. Much of the detail is also contained within the diff itself.
The tooltip behaviour on the file name (see below) is technically an accident that it appears right over top of the tile, but I think it works out okay enough to not be concerned.
Designers: Instead of truncating to 20-30 characters, this truncates to roughly 15 characters so that the file size has a chance at fitting in the tile at all times.
Notes: none
element-web notes: none