Fix download button styling in multiselection dialogue #1684
Fix download button styling in multiselection dialogue #1684Saira-A wants to merge 3 commits intoUniversalViewer:devfrom
Conversation
Fixes text only showing on hover and changes text label
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@github-actions[bot] is attempting to deploy a commit to the BL UV team Team on Vercel. A member of the Team first needs to authorize it. |
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @Saira-A, I was able to use your sample manifest successfully. Was download previously renamed to select and is now being renamed back? I just want to be sure I understand the context of these changes, since in theory the rename could impact people's customizations (though in practice, I think it's relatively unlikely that people have customized this particular thing).
See below for other questions/observations:
| select: "Select", | ||
| selectAll: "Select All", | ||
| download: "Download", | ||
| selectAll: "Select All ", |
There was a problem hiding this comment.
Why the here? If there's an issue with the checkbox label and download button being too close together, can we use CSS to fix it instead?
Also, I don't think this area of the code is correctly internationalized; at least, when I switch the UI to Welsh, these parts of the dialog box remain in English.
2 participants
This PR renames the "select" button in the download selection gallery dialogue back to "download", which was the original label, for clarity. It also fixes a styling issue where that text only showed while hovering over the button.
This requires a manifest with selectionEnabled, e.g https://gist.githubusercontent.com/Saira-A/988dcfde9afc2971c842ae0d3392a91d/raw/30a5ea0d3b7595e3cbb40f17503c3d3f21064513/gistfile1.json (no download service so the button won't actually work).
Others have had issues getting that manifest to work so I've also included screenshots below. The "download" and "select all" labels are hard-coded in so that also needs to be fixed so will do later if needed (looks like only the BL used this feature and we might not bring it back anyway).