Skip to content

Commit

Permalink
adding more writable checks
Browse files Browse the repository at this point in the history
  • Loading branch information
dharmaquark committed Sep 19, 2018
1 parent 9256af3 commit 11e1c83
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
14 changes: 12 additions & 2 deletions packages/docmanager-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,13 +522,23 @@ function addCommands(
isEnabled: () => {
const iterator = shell.widgets('main');
let widget = iterator.next();
let anyWritable = false;
while (widget) {
if (docManager.contextForWidget(widget)) {
return true;
let context = docManager.contextForWidget(widget);

This comment has been minimized.

Copy link
@jasongrout

jasongrout Sep 24, 2018

Contributor

Perhaps we could set the context variable first, then check to see if context is writable, so we don't have two calls to contextForWidget:

let context = docManager.contextForWidget(widget)
if (context && context.contentsModel && context.contentsModel.writable) { ... }
if (
context &&
context.contentsModel &&
context.contentsModel.writable
) {
anyWritable = true;

This comment has been minimized.

Copy link
@ivanov

ivanov Sep 19, 2018

Member

Does it make sense to just shortcut and skip checking all the other widgets and return true; here?

}
}
widget = iterator.next();
}
return false;
// disable saveAll if all of the widgets models
// have writable === false
return anyWritable;

This comment has been minimized.

Copy link
@ivanov

ivanov Sep 19, 2018

Member

Then we can keep the return false; here and not need the anyWriteable variable.

},
execute: () => {
const iterator = shell.widgets('main');
Expand Down
3 changes: 2 additions & 1 deletion packages/notebook/src/default-toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export namespace ToolbarItems {
}
});
},
tooltip: 'Save the notebook contents and create checkpoint'
tooltip: 'Save the notebook contents and create checkpoint',
enabled: panel.context.contentsModel.writable
});
}

Expand Down

0 comments on commit 11e1c83

Please sign in to comment.