Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ENYO-1020: implement "Save As" under the Text Editor #243

Merged
merged 23 commits into from Mar 29, 2013

Conversation

Projects
None yet
2 participants
Owner

asnowfix commented Mar 29, 2013

Tested on OSX/ChromeCanary, OSX/Opera (so far), OSX/Firefox (SaveAs works , not Save, due to https://enyojs.atlassian.net/browse/ENYO-1269)

  • ENYO-1020: update FS protocol description, following @opopova comment
  • ENYO-1020: remove dead code (Documents.js)
  • ENYO-1020: implement Phobos#saveAs
  • ENYO-1020: refactor FileChooser to be a filePicker or a folderPicker
  • ENYO-1020: share waitPopup between Ares & Phobos
  • ENYO-1996: Added getParentOfSelected method on HermesFileTree
  • ENYO-1020: add ares.basename() and ares.dirname() utilities
  • ENYO-2104: remove misleading runares.bat
  • ENYO-1020: add /favicon.ico (avoid 404 in the logs...)
  • ENYO-1020: Move top-level Ares MVC components under ares/source/ (2/2)
  • ENYO-2104: refresh Ares usage instructions
  • ENYO-1020: Move top-level Ares MVC components under ares/source/
  • Merge remote-tracking branch 'enyojs/master' into ENYO-1020
  • ENYO-1020: DirectorySelectorPopup is now FileChooser
  • ENYO-1020: Fix typo in EditorSettings breaking the chrome theme
  • ENYO-1020: editor: turn Save button into File menu…

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI francois-xavier.kowalski@hp.com

asnowfix and others added some commits Mar 20, 2013

ENYO-1020: editor: turn `Save` button into `File` menu…
- `File` menu has `Save`, `Save as` and `Close` menu items.

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: Fix typo in `EditorSettings` breaking the `chrome` theme
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: `DirectorySelectorPopup` is now `FileChooser`
- `DirectorySelectorPopup` is now `Ares.FileChooser` defined by
  `utilities/source/FileChooser`
- Move manual `style:`  into `Ares.css` as `.ares-filechooser*`

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: Move top-level Ares MVC components under ares/source/
- the `model/` tree was not attached to one of the functional Ares
  components (Ares, Harmonia, Phobos, Deimos)

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-2104: refresh Ares usage instructions
- git clone is no longer the primary way of delivering Ares

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: Move top-level Ares MVC components under ares/source/ (2/2)
- fix broken package.js

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: add /favicon.ico (avoid 404 in the logs...)
- remove former ares.png files fetched from the Net with random license
- add icon from Ares1 (https://ares.palm.com) as favicon.ico

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-2104: remove misleading runares.bat
- prefer to use NPM-generated wrappers

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: add `ares.basename()` and `ares.dirname()` utilities
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: share waitPopup between Ares & Phobos
- Former implementation fires up one, the other one or both without
  a well-defined (or understandable) logic.
- New implementation uses a single Ares#waitPopup that is usable via
  events by any module of Ares.

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: refactor FileChooser to be a filePicker or a folderPicker
- Still used by ProjectWizard (folderPicker)
- Ready to be used by Phobos#saveAs (filePicker)

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: implement Phobos#saveAs
- UI interaction is handled by Phobos (text editor), which raises events
- Events are trapped at the Ares level, which implements async file-
  system operations & report their success back to Phobos
- FIXME: Phobos#saveAs() uses a common-JS type of completion callback
  (which is good) while Phobos#save() makes Ares directly call into
  Phobos's API (which is ugly, because Ares needs to know about an async
  operation completion public API which does not need to exist).
- FIXME: saveAs popup does not show the folder where the current file
  is.  This is Ok in case of multiple saveAs (operation resumes in the
  same folder as before), but not for the first popup.

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: remove dead code (Documents.js)
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: update FS protocol description, following @opopova comment
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>

@ghost ghost assigned dod38fr Mar 29, 2013

ENYO-1020: decrease FileChooser tracing level.
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
Member

dod38fr commented Mar 29, 2013

File 'saved as...' is not shown in HermesFileTree of the project. HermesFileTree should be refreshed once the file is saved

Member

dod38fr commented Mar 29, 2013

tested on:

  • Debian with Chromium and firefox 10.
  • Windows7 with IE9

@dod38fr dod38fr commented on the diff Mar 29, 2013

ares/Ares.css
@@ -28,6 +28,24 @@ body {
}
/**/
+
+.ares-filechooser {
+ height: 400px;
+ width: 600px;
+}
+
+.ares-filechooser-header {
@dod38fr

dod38fr Mar 29, 2013

Member

why 2 classes with the same properties ?

@asnowfix

asnowfix Mar 29, 2013

Owner

Although they have the same properties now (and their visual look is equally ugly), they are not use at the same place & I expect them to evolve differently as @opopova will work on them.

Member

dod38fr commented Mar 29, 2013

There's no attributions for the new icons. See this example of attribution

@dod38fr dod38fr commented on an outdated diff Mar 29, 2013

ares/source/Ares.js
+ } else if (!file.isDir && !name) {
+ // overwrite the given file
+ where = {
+ service: file.service,
+ fileId: file.id
+ };
+ } else if (!file.isDir && name) {
+ // create a new file in the same folder as the
+ // given file
+ where = {
+ service: file.service,
+ folderId: file.parent.id,
+ name: name
+ };
+ } else {
+ err = new Error("wrong file/folder description");
@dod38fr

dod38fr Mar 29, 2013

Member

maybe mention that's an internal error.

@dod38fr dod38fr commented on an outdated diff Mar 29, 2013

ares/source/Ares.js
+ },
+ saveAsDocument: function(inSender, inEvent) {
+ if (this.debug) this.log("sender:", inSender, ", event:", inEvent);
+ var self = this,
+ file = inEvent.file,
+ name = inEvent.name,
+ content = inEvent.content;
+
+ if (!file) {
+ _footer(new Error("missing file/folder description"));
+ return;
+ }
+
+ async.waterfall([
+ this._closeDocument.bind(this, inEvent.docId),
+ function(next) {
@dod38fr

dod38fr Mar 29, 2013

Member

the purpose of this anonymous function is not easy to understand. You should either add a comment or put this function in a local variable with a descriptive name. The latter is preferred as the waterfall sequence would be easier to read

@dod38fr dod38fr commented on the diff Mar 29, 2013

@@ -427,6 +427,8 @@ var addr = argv.host;
app.configure(function(){
+ app.use(express.favicon(__dirname + '/ares/assets/images/ares_48x48.ico'));
@dod38fr

dod38fr Mar 29, 2013

Member

__dirname is undef in node-webkit, use the myDir variable.

ENYO-2089: refresh the FileTree after `Save As…`
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>

@dod38fr dod38fr commented on an outdated diff Mar 29, 2013

utilities/source/FileChooser.js
+ {name: "folderSelector", kind: "onyx.InputDecorator", classes: "onyx-toolbar-inline", components: [
+ {content: $L("Where") + ":", fit: true},
+ {name: "selectedFolder", kind: "onyx.Input", classes: "only-light", disabled: true, placeholder: $L("Folder")}
+ ]},
+ {name: "nameSelector", kind: "onyx.InputDecorator", classes: "onyx-toolbar-inline", showing: false, components: [
+ {content: $L("As") + ":", fit: true},
+ {name: "selectedName", kind: "onyx.Input", classes: "only-light", disabled: true, placeholder: $L("File"), selectOnFocus: true, onchange: "updateSelectedName"}
+ ]},
+ {name: "cancelButton", kind: "onyx.Button", classes: "onyx-negative", content: $L("Cancel"), ontap: "cancel"},
+ {name: "confirmButton", kind: "onyx.Button", classes: "onyx-affirmative", content: $L("OK"), ontap: "confirm"}
+ ]}
+ ]}
+ ],
+ events: {
+ /**
+ * Emitted once the end-user clicks cnacel or when a
Owner

asnowfix commented Mar 29, 2013

Just stacked dc8be21 fixing the auto-refresh issue.

@dod38fr dod38fr commented on the diff Mar 29, 2013

utilities/source/FileChooser.js
+ this.hide() ;
+ if (this.debug) this.log("selectedFile:", this.selectedFile, "selectedName:", this.selectedName);
+ this.doFileChosen({
+ file: this.selectedFile,
+ name: (this.selectedName === this.selectedFile.name ? undefined : this.selectedName)
+ });
+ return true; // Stop event propagation
+ },
+ /** @private */
+ cancel: function(inSender, inEvent) {
+ if (this.debug) this.log("sender:", inSender, ", event:", inEvent);
+ this.hide() ;
+ this.doFileChosen();
+ return true; // Stop event propagation
+ },
+ //FIXME: This is *nearly* identical to the code in Harmonia. Maybe move this into HermesFileTree?
@dod38fr

dod38fr Mar 29, 2013

Member

In Enyo-1996, Harmonia is now a very thin shell. All the file and dir handling was moved in HermesFileTree

@asnowfix

asnowfix Mar 29, 2013

Owner

To be adapted along when ENYO-1996 comes in, then.

ENYO-2089: review comment: add Ares icon attribution
- Also fix wiki formatting of former attribution (from open clipart)

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
Owner

asnowfix commented on 7c0f3dd Mar 29, 2013

Added icon attribution: 7c0f3dd. We have a fairly large Palm/HP-owned set of icons with Ares 1. I prefer to not use external source as much as possible.

asnowfix added some commits Mar 29, 2013

ENYO-1020: fix review comment (typo)
Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: fix bug introduced while addressing review comment
- Use `refreshFile`, not `refreshFileTree`

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
ENYO-1020: address review comment (clarify)
- turn lambda function used in `async.waterfall` into local a nested one
  with a (hopefully descriptive) name `_prepareNewLocation`
- Use `refreshFile`, not `refreshFileTree`

Enyo-DCO-1.1-Signed-off-by: Francois-Xavier KOWALSKI <francois-xavier.kowalski@hp.com>
Owner

asnowfix commented Mar 29, 2013

@dod38fr : review comments addressed. ready to merge.

dod38fr added a commit that referenced this pull request Mar 29, 2013

Merge pull request #243 from enyojs/ENYO-1020
ENYO-1020: implement "Save As" under the Text Editor

Re-tested on Debian/Chromium. Looks good

@dod38fr dod38fr merged commit b853554 into master Mar 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment