-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Live Preview server-side file support #2004
Conversation
@jasonsanjose - ready for review |
var NativeApp = require("utils/NativeApp"); | ||
var Dialogs = require("widgets/Dialogs"); | ||
var Strings = require("strings"); | ||
var StringUtils = require("utils/StringUtils"); | ||
var ProjectManager = require("project/ProjectManager"); | ||
var PreferencesDialogs = require("preferences/PreferencesDialogs"); |
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.
Can we clean this up to a single var decl?
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.
Hope I didn't go overboard :) I also lined up blocks and put lists in alphabetic order.
Initial review complete |
Changes pushed. Ready for re-review. |
I forgot to test this the first time, but I'm seeing the URL encoding issue #2003 when using an encoded URL, e.g. EDIT: Oops, I see that was listed under known issues. Ignore. |
Merging. |
Live Preview server-side file support
} | ||
|
||
if (_isHtmlFileExt(doc.extension)) { |
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're dropping one condition here and I think this is the culprit for the unit test failing.
You need to AND exports.config.experimental condition with your _isHtmlFileExt call.
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.
This user story implements extensions other than .htm/.html, so it not experimental anymore.
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.
Believe me on this. We're not usig HTMLDocument yet. It is still in experimental stage and therefore we were not creating it before this change. If you create an HTML document, then it will interfere with DOMAgent which is currently in use.
Anyway, have you tried it out to see it fixed the unit tests?
New code:
Cleanup of existing code
Known Issues