-
Notifications
You must be signed in to change notification settings - Fork 7.6k
fix #5686 - [preview image] Some display issues for image info in preview image. #5877
Conversation
1. make image meta data strings not wrap 2. add ellipsis to image meta data when clipping 3. add title attribute to allow viewing the full path and meta data when text is clipped.
@@ -64,7 +64,9 @@ define(function (require, exports, module) { | |||
currentPath = $("#img-path").text(); | |||
|
|||
if (currentPath === oldRelPath) { | |||
$("#img-path").text(ProjectManager.makeProjectRelativeIfPossible(newName)); | |||
var newRelName = ProjectManager.makeProjectRelativeIfPossible(newName); | |||
$("#img-path").text(newRelName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to use html
and not text
since file path can have some encoded characters for high-ascii and double-byte characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it, but my intent was to add a tooltip. Since code was using text before I did not change it.
Done with initial review. |
Done with changes per review feedback by @RaymondLim . |
}, | ||
function (error) { | ||
$("#img-data").html(dimensionString); | ||
$("#img-data").html(dimensionString) | ||
.attr("title", dimensionString).replace("×", "x"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have one close paren in the wrong place. You meant to call replace for dimensionString, not chaining replace call to attr call. ie. .attr("title", dimensionString.replace("×", "x"));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yikes. Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed fix
Ping @RaymondLim ! Any other changes? |
Sorry to say that I suggested the wrong thing -- changing from .text to .html. That breaks all the file path with some characters like |
Oh - ok. |
I changed the path assignments back to use text rather than html |
I'll also merge master in a sec. |
Conflicts: src/editor/ImageViewer.js
Merging. |
fix #5686 - [preview image] Some display issues for image info in preview image.
The following changes address #1 of bug #5686: When resizing the width of the window smaller, the top two lines of text will wrap. I don't think we should wrap these two lines of image info, just let them being clipped if they don't fit inside the window width.
This pull requests makes the following changes