Cleaned up ViewImageModal and FileAttachment and added some extra comments

3 files changed, 27 insertions, 11 deletions

diff --git a/web/react/components/file_attachment.jsx b/web/react/components/file_attachment.jsx
index 57a064ff4..3cd791887 100644
--- a/web/react/components/file_attachment.jsx
+++ b/web/react/components/file_attachment.jsx
@@ -7,9 +7,13 @@ module.exports = React.createClass({
     displayName: "FileAttachment",
     canSetState: false,
     propTypes: {
+        // a list of file pathes displayed by the parent FileAttachmentList
         filenames: React.PropTypes.arrayOf(React.PropTypes.string).isRequired,
+        // the index of this attachment preview in the parent FileAttachmentList
         index: React.PropTypes.number.isRequired,
+        // the identifier of the modal dialog used to preview files
         modalId: React.PropTypes.string.isRequired,
+        // handler for when the thumbnail is clicked
         handleImageClick: React.PropTypes.func
     },
     getInitialState: function() {
@@ -20,12 +24,8 @@ module.exports = React.createClass({
 
         var filename = this.props.filenames[this.props.index];
 
-        var self = this;
-
         if (filename) {
             var fileInfo = utils.splitFileLocation(filename);
-            if (Object.keys(fileInfo).length === 0) return;
-
             var type = utils.getFileType(fileInfo.ext);
 
             // This is a temporary patch to fix issue with old files using absolute paths
@@ -35,6 +35,7 @@ module.exports = React.createClass({
             fileInfo.path = utils.getWindowLocationOrigin() + "/api/v1/files/get" + fileInfo.path;
 
             if (type === "image") {
+                var self = this;
                 $('<img/>').attr('src', fileInfo.path+'_thumb.jpg').load(function(path, name){ return function() {
                     $(this).remove();
                     if (name in self.refs) {
@@ -87,6 +88,7 @@ module.exports = React.createClass({
             thumbnail = <div className={"file-icon "+utils.getIconClassName(type)}/>;
         }
 
+        var fileSizeString = "";
         if (this.state.fileSize < 0) {
             var self = this;
 
@@ -96,10 +98,7 @@ module.exports = React.createClass({
                     self.setState({fileSize: fileSize});
                 }
             });
-        }
-
-        var fileSizeString = "";
-        if (this.state.fileSize >= 0) {
+        } else {
             fileSizeString = utils.fileSizeToString(this.state.fileSize);
         }
 
diff --git a/web/react/components/file_attachment_list.jsx b/web/react/components/file_attachment_list.jsx
index 5f2690fdf..b92442957 100644
--- a/web/react/components/file_attachment_list.jsx
+++ b/web/react/components/file_attachment_list.jsx
@@ -8,9 +8,13 @@ var Constants = require('../utils/constants.jsx');
 module.exports = React.createClass({
     displayName: "FileAttachmentList",
     propTypes: {
+        // a list of file pathes displayed by this
         filenames: React.PropTypes.arrayOf(React.PropTypes.string).isRequired,
+        // the identifier of the modal dialog used to preview files
         modalId: React.PropTypes.string.isRequired,
+        // the channel that this is part of
         channelId: React.PropTypes.string,
+        // the user that owns the post that this is attached to
         userId: React.PropTypes.string
     },
     getInitialState: function() {
diff --git a/web/react/components/view_image.jsx b/web/react/components/view_image.jsx
index c7fb1b785..4b2f8f650 100644
--- a/web/react/components/view_image.jsx
+++ b/web/react/components/view_image.jsx
@@ -5,6 +5,8 @@ var Client = require('../utils/client.jsx');
 var utils = require('../utils/utils.jsx');
 
 module.exports = React.createClass({
+    displayName: "ViewImageModal",
+    canSetState: false,
     handleNext: function() {
         var id = this.state.imgId + 1;
         if (id > this.props.filenames.length-1) {
@@ -90,6 +92,12 @@ module.exports = React.createClass({
                 $(self.refs.imageFooter.getDOMNode()).removeClass("footer--show");
             }
         );
+
+        // keep track of whether or not this component is mounted so we can safely set the state asynchronously
+        this.canSetState = true;
+    },
+    componentWillUnmount: function() {
+        this.canSetState = false;
     },
     getPublicLink: function(e) {
         data = {};
@@ -130,6 +138,7 @@ module.exports = React.createClass({
             var fileType = utils.getFileType(fileInfo.ext);
 
             if (fileType === "image") {
+                // image files just show a preview of the file
                 content = (
                     <a href={fileUrl} target="_blank">
                         <img ref="image" src={this.getPreviewImagePath(filename)}/>
@@ -160,13 +169,16 @@ module.exports = React.createClass({
                     var self = this;
 
                     utils.getFileSize(utils.getFileUrl(filename), function(fileSize) {
-                        var fileSizes = self.state.fileSizes;
-                        fileSizes[filename] = fileSize;
-                        self.setState(fileSizes);
+                        if (self.canSetState) {
+                            var fileSizes = self.state.fileSizes;
+                            fileSizes[filename] = fileSize;
+                            self.setState(fileSizes);
+                        }
                     });
                 }
             }
         } else {
+            // display a progress indicator when the preview for an image is still loading
             var percentage = Math.floor(this.state.progress[this.state.imgId]);
             content = (
                 <div>
@@ -216,6 +228,7 @@ module.exports = React.createClass({
             </div>
         );
     },
+    // Returns the path to a preview image that can be used to represent a file.
     getPreviewImagePath: function(filename) {
         var fileInfo = utils.splitFileLocation(filename);
         var fileType = utils.getFileType(fileInfo.ext);