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

Display informational warning when code is edited while its running #35351

Merged
merged 21 commits into from
Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/i18n/common/en_us.json
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,7 @@
"edit": "Edit",
"editAll": "Edit all",
"editProject": "Edit Project",
"editDuringRunMessage": "Your code may have changed. Click \"Reset\" and then \"Run\" to run your code again.",
"editSectionDetails": "Edit Section Details",
"editSectionLoginTypeCleverDesc": "Students sign in through Clever.",
"editSectionLoginTypeEmailDesc": "Students sign in with their personal email login information.",
Expand Down
100 changes: 81 additions & 19 deletions apps/src/StudioApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ class StudioApp extends EventEmitter {
* Levelbuilder-defined helper libraries.
*/
this.libraries = {};

this.editDuringRunAlert = undefined;
this.executingCode = undefined;
JillianK marked this conversation as resolved.
Show resolved Hide resolved
}
}
/**
Expand Down Expand Up @@ -914,6 +917,20 @@ StudioApp.prototype.addChangeHandler = function(newHandler) {
};

StudioApp.prototype.runChangeHandlers = function() {
if (
this.isRunning() &&
this.editDuringRunAlert === undefined &&
(this.isUsingBlockly() ||
this.getCode().trim() !== this.executingCode.trim())
JillianK marked this conversation as resolved.
Show resolved Hide resolved
) {
// we trim the whitespace from the start and end of the code before comparing in droplet
// because droplet sometimes adds an extra newline when switching from block to code mode
this.editDuringRunAlert = this.displayWorkspaceAlert(
'warning',
<div>{msg.editDuringRunMessage()}</div>,
true
);
}
JillianK marked this conversation as resolved.
Show resolved Hide resolved
if (!this.changeHandlers) {
return;
}
Expand Down Expand Up @@ -944,6 +961,15 @@ StudioApp.prototype.toggleRunReset = function(button) {

getStore().dispatch(setIsRunning(!showRun));

if (showRun) {
if (this.editDuringRunAlert !== undefined) {
ReactDOM.unmountComponentAtNode(this.editDuringRunAlert);
this.editDuringRunAlert = undefined;
}
} else {
this.executingCode = this.getCode().trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

Still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change handlers are also run when you toggle between code and block mode in droplet so we need the code when the user clicked run to compare against later to see if there was an actual change.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}

if (this.hasContainedLevels) {
lockContainedLevelAnswers();
}
Expand Down Expand Up @@ -3039,25 +3065,49 @@ function rectFromElementBoundingBox(element) {
* @param {string} type - Alert type (error, warning, or notification)
* @param {React.Component} alertContents
*/
StudioApp.prototype.displayWorkspaceAlert = function(type, alertContents) {
var container = this.displayAlert(
'#codeWorkspace',
{type: type},
alertContents
);
StudioApp.prototype.displayWorkspaceAlert = function(
JillianK marked this conversation as resolved.
Show resolved Hide resolved
type,
alertContents,
bottom = false
) {
let container;
if (bottom) {
container = this.displayAlert(
this.editCode ? '#codeTextbox' : '#codeWorkspace',
{type: type, sideMargin: 0, bottomMargin: 0, bottom: true},
alertContents
);
} else {
container = this.displayAlert(
'#codeWorkspace',
{type: type},
alertContents
);
}

var toolbarWidth;
if (this.usingBlockly_) {
if (this.usingBlockly_ && this.config.app === 'craft') {
// craft has a slightly different way of constructing the toolbox so we need to use
// the toolbox header's width to get the width of the actual toolbox.
toolbarWidth = $('#toolbox-header').width();
} else if (this.usingBlockly_) {
toolbarWidth = $('.blocklyToolboxDiv').width();
} else {
toolbarWidth =
$('.droplet-palette-element').width() + $('.droplet-gutter').width();
}

$(container).css({
left: toolbarWidth,
top: $('#headers').height()
});
if (bottom) {
$(container).css({
left: toolbarWidth
});
} else {
$(container).css({
left: toolbarWidth,
top: $('#headers').height()
});
}
return container;
};

/**
Expand Down Expand Up @@ -3114,14 +3164,25 @@ StudioApp.prototype.displayAlert = function(
var parent = $(selector);
var container = parent.children('.react-alert');
if (container.length === 0) {
container = $("<div class='react-alert ignore-transform'/>").css({
position: position,
left: 0,
right: 0,
top: 0,
zIndex: 1000,
transform: 'scale(1.0)'
});
if (props.bottom) {
container = $("<div class='react-alert ignore-transform'/>").css({
position: position,
left: 0,
right: 0,
bottom: 0,
zIndex: 1000,
transform: 'scale(1.0)'
});
JillianK marked this conversation as resolved.
Show resolved Hide resolved
} else {
container = $("<div class='react-alert ignore-transform'/>").css({
position: position,
left: 0,
right: 0,
top: 0,
zIndex: 1000,
transform: 'scale(1.0)'
});
}
parent.append(container);
}
var renderElement = container[0];
Expand All @@ -3134,6 +3195,7 @@ StudioApp.prototype.displayAlert = function(
onClose={handleAlertClose}
type={props.type}
sideMargin={props.sideMargin}
bottomMargin={props.bottomMargin}
closeDelayMillis={props.closeDelayMillis}
childPadding={props.childPadding}
>
Expand Down
3 changes: 2 additions & 1 deletion apps/src/templates/alert.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default class Alert extends React.Component {
onClose: PropTypes.func.isRequired,
closeDelayMillis: PropTypes.number,
sideMargin: PropTypes.number,
bottomMargin: PropTypes.number,
childPadding: PropTypes.string
};

Expand Down Expand Up @@ -45,7 +46,7 @@ export default class Alert extends React.Component {
child: {
// from bootstrap's alert
padding: valueOr(this.props.childPadding, '8px 35px 8px 14px'),
marginBottom: 20,
marginBottom: valueOr(this.props.bottomMargin, 20),
textShadoow: '0 1px 0 rgba(255, 255, 255, 0.5)',
border: '1px solid',
borderRadius: 4
Expand Down
11 changes: 9 additions & 2 deletions apps/test/unit/StudioAppTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,15 @@ describe('StudioApp', () => {
});

describe('addChangeHandler', () => {
beforeEach(stubStudioApp);
afterEach(restoreStudioApp);
beforeEach(() => {
stubStudioApp();
stubRedux();
registerReducers(commonReducers);
});
afterEach(() => {
restoreRedux();
restoreStudioApp();
});

it('calls a handler in response to a blockly change', () => {
let changed = false;
Expand Down
Binary file modified dashboard/db/schema_cache.dump
Binary file not shown.