[ICU-648] Fix download tooltip #692
Conversation
saturninoabril
added
1: PM Review
| iconClass={'post-image__download'} | ||
| index={index} | ||
| > | ||
| <DownloadIcon/> |
There was a problem hiding this comment.
This is the actual fix. Others are just refactor.
There was a problem hiding this comment.
@saturninoabril can we please,
- Move the tooltip to just above the download button. It looks too far away/disconnected right now. @asaadmahmood might be able to help if necessary.
- Change the text to just "Download" (without the file name)
|
@esethna Thanks for the review. I have addressed both 1 & 2. See screenshots below.
|
components/file_attachment.jsx
Outdated
| @@ -40,7 +47,7 @@ export default class FileAttachment extends React.PureComponent { | |||
| super(props); | |||
|
|
|||
| this.state = { | |||
| loaded: Utils.getFileType(props.fileInfo.extension) !== 'image' | |||
| loaded: getFileType(props.fileInfo.extension) !== 'image' | |||
There was a problem hiding this comment.
Not part of this PR, I know, but it drives me crazy that getFileType doesn't return constant-defined strings that could be compared against. Would be great if someone refactored that...
There was a problem hiding this comment.
Yeah, that makes sense, I'll submit separate PR to refactor that.
|
Rebased to latest and fixed merge conflicts |
| /** | ||
| * File URL | ||
| */ | ||
| fileURL: PropTypes.string |
There was a problem hiding this comment.
Are these props really not required? Just verifying as per last dev meeting.
utils/file_utils.jsx
Outdated
|
|
||
| export function trimFilename(filename) { | ||
| let trimmedFilename = filename; | ||
| if (filename.length > 35) { |
There was a problem hiding this comment.
Can we define this magic 35 somewhere?
saturninoabril
added
4: Reviews Complete
GoldUniform
removed
the
4: Reviews Complete
amyblais
added
Changelog/Not Needed
Summary
Move tooltip from filename to download icon
Others:
FileAttachmentcomponentFileThumbnailcomponent@esethna Could you please check if the tooltip is positioned as you would expect? Thanks!
Update (Feb 2):
Ticket Link
This PR addressed item 1 of:
Checklist
make check-styleto check for style errors (required for all pull requests)make testto ensure unit and component tests passed