Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/save load gist #675

Merged
merged 27 commits into from
Dec 12, 2016
Merged

Feature/save load gist #675

merged 27 commits into from
Dec 12, 2016

Conversation

joshuafcole
Copy link
Contributor

@joshuafcole joshuafcole commented Dec 9, 2016

Enables

  • Saving current document as markdown to gist
  • Loading documents from gists in-editor
  • Loading documents from gist-formatted file ids in the editor.

Blocking tasks:

  • Silently remap local anchor tag hrefs to ignore the IDE portion of the document fragment. This isn't any more broken than it used to be, but the problem becomes more prevalent since the documentId is no longer a known static value.
  • Handle saving edits to documents loaded from gists in some reasonable way.

Notes:

  • Code quality is lacking here. Improvements are blocked by some refactoring work that isn't critical. it's still worth discussing any specific issues found, but it's important that we revisit this in the near future for cleanup as part of the broader file store rework.

@ibdknox
Copy link
Contributor

ibdknox commented Dec 9, 2016

Is this able to load regular github links yet?

@ibdknox
Copy link
Contributor

ibdknox commented Dec 9, 2016

Seems to work fine for gists :) I think we probably want to make some changes here to make it a little more obvious UX-wise:

  • We should get rid of my on hover for hiding for the nav so that it's always there
  • The load button at least should show up on the directory part of the nav
  • The saved to gist notification should probably be a bit more noticable
  • It'd be nice to have some feedback when you initially press the save button that lets you know it's doing something
  • Collapsing the nav now puts the arrow in a strange place

//console.error("Cannot load non-existent document.");
//return;

root = this.nodes[id] = {id, type: "document", name};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this always right? What happens if I do try to load something non-existent locally?

{c: `${this.open ? "expand-btn" : "collapse-btn"} ion-ios-arrow-back`, click: this.togglePane},

this.open && type === "document" ? {c: "ion-ios-cloud-upload-outline btn", title: "Save to Gist", click: () => this.ide.saveToGist()} : undefined,
this.open && type === "document" ? {c: "ion-ios-cloud-download-outline btn", title: "Load from Gist", click: this.openLoadDialog} : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like I should be able to load a gist regardless of where I am

@joshuafcole
Copy link
Contributor Author

@ibdknox thoughts on making the saved notice more visible without being obnoxious?

@ibdknox
Copy link
Contributor

ibdknox commented Dec 10, 2016

Nav is empty when:

  1. Load, change, load, change, click overwrite

On change the new file doesn't show up in the directory index without a refresh.

@joshuafcole joshuafcole mentioned this pull request Dec 10, 2016
@joshuafcole
Copy link
Contributor Author

This is ready to ship pending final approval.

Copy link
Contributor

@ibdknox ibdknox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@joshuafcole joshuafcole merged commit a29052d into master Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants