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

Trigger inspection on cursor movement #5899

Closed
ian-r-rose opened this issue Jan 23, 2019 · 4 comments
Closed

Trigger inspection on cursor movement #5899

ian-r-rose opened this issue Jan 23, 2019 · 4 comments

Comments

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 23, 2019

We currently trigger inspection requests when the text content of an editor with a kernel attached to it changes. This misses cases where the user is navigating the cursor without making edits, or when they select a new editor (e.g., a new cell).

It would be nice to also trigger inspection for those events. We would have to go to the inspection handler and wire up more editor signals to a new text editor here:

if (this._editor && !this._editor.isDisposed) {
this._editor.model.value.changed.disconnect(this.onTextChanged, this);
}
let editor = (this._editor = newValue);
if (editor) {
// Clear the inspector in preparation for a new editor.
this._cleared.emit(void 0);
editor.model.value.changed.connect(
this.onTextChanged,
this

In particular, in order to trigger inspection on cursor movement, we would listen to the editor.model.selections.changed signal. It may also be useful to wire up an ActivityMonitor to throttle the rate of network requests. An example of this is here:

// Throttle the rendering rate of the widget.
this._monitor = new ActivityMonitor({
signal: this._context.model.contentChanged,
timeout: options.renderTimeout
});
this._monitor.activityStopped.connect(
this.update,
this
);

Marking this as a good issue for a new contributor.

@ntdef
Copy link
Contributor

@ntdef ntdef commented Jan 25, 2019

Were you expecting that it should call

protected onTextChanged(): void {

on editor.model.selections.changed? Does that mean that the function name should change as well?

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Jan 25, 2019

Yeah, that's what I had in mind. Maybe something like onEditorChange? What do you think?

@ntdef
Copy link
Contributor

@ntdef ntdef commented Jan 25, 2019

@ntdef
Copy link
Contributor

@ntdef ntdef commented Jan 25, 2019

Just put out a PR.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants