Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Conversation

@dwillmer
Copy link
Member

No description provided.

src/index.ts Outdated

Choose a reason for hiding this comment

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

The view model should not know about the view. What's the intent of this relationship?

Copy link
Member Author

Choose a reason for hiding this comment

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

Codemirror has an .on("change",...) event which is re-emitted here as a phosphor-signal. The view-model connects to that signal, so requires a handle on the view (at least temporarily).

Choose a reason for hiding this comment

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

That's not the responsibility of the view model. That connection should be handled in the view, and converted to a method call on the view model. i.e. move the timer to the widget.

@sccolbert
Copy link

I'm not sure separating model and view model in this case makes a lot of sense. I think we can combine IEditorModel and IEditorViewModel into just IEditorModel. It's technically a view model, but that's just semantics.

src/index.ts Outdated

Choose a reason for hiding this comment

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

If you want to make this is an interface (and I don't think we should) it needs to be interface IEditorWidget extends Widget

README.md Outdated

Choose a reason for hiding this comment

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

this is not a supported runtime - Node doesn't have a DOM.

Choose a reason for hiding this comment

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

I would double check the rest of the README to make sure it's accurate.

@sccolbert
Copy link

ping @Carreau

@sccolbert
Copy link

We need your input on what you think an EditorModel needs to have

@sccolbert
Copy link

@dwillmer LGTM for a first cut (modulo comments). I'm assuming we'll iterate on this.

@sccolbert
Copy link

ping @jasongrout You seem to have done some interface work for input area/editor. I think you and Dave should sync up.

blink1073 added a commit that referenced this pull request Nov 16, 2015
Initial version with example
@blink1073 blink1073 merged commit 527cf4f into jupyter:master Nov 16, 2015
blink1073 added a commit that referenced this pull request Jan 9, 2016
add github-flavored markdown to the modes the bundler will pick up automatically
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants