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

Stop function final reviwe #109

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions Browser_IDE/editorMain.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,15 +809,18 @@ async function pauseProgram(){
}
}

async function stopProgram(){
async function stopProgram() {
try {
await executionEnviroment.stopProgram();
}
catch (err) {
displayEditorNotification("Failed to stop program!", NotificationIcons.ERROR);
await executionEnviroment.cleanEnvironment(); // Clear resources or locks
displayEditorNotification("Program has been stopped successfully.", NotificationIcons.INFO);
} catch (err) {
displayEditorNotification("Failed to stop program!<br/>" + err.toString(), NotificationIcons.ERROR);
}
}



async function restartProgram(){
clearErrorLines();

Expand Down
92 changes: 91 additions & 1 deletion Browser_IDE/executionEnvironment.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,4 +312,94 @@ class ExecutionEnvironment extends EventTarget{
return iframe;
}

}
}

document.getElementById("stopProgram").addEventListener("click", () => {
terminateProgram();
});

document.getElementById("collapsedStopProgram").addEventListener("click", () => {
terminateProgram();
});

// Attach event listeners for Stop buttons
initializeEventListeners();

function initializeEventListeners() {
const stopButton = document.getElementById("stopProgram");
const collapsedStopButton = document.getElementById("collapsedStopProgram");

stopButton.addEventListener("click", handleStopProgram);
collapsedStopButton.addEventListener("click", handleStopProgram);
}

/**
* Handles the stop button click event
*/
function handleStopProgram() {
if (!isProgramRunning()) {
showNotification("No program is currently running.", "warning");
return;
}

try {
terminateProgram();
showNotification("Program stopped successfully.", "info");
} catch (error) {
console.error("Error while stopping the program:", error);
showNotification("Failed to stop the program. Please try again.", "error");
}
}

/**
* Terminates the currently running program
*/
function terminateProgram() {
try {
// Logic to terminate the running process
runtimeManager.terminate();
updateRuntimeButtons(false); // Disable runtime buttons after stopping
} catch (error) {
throw new Error("Program termination failed: " + error.message);
}
Comment on lines +354 to +365
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, AI code that shouldn't be in here.

}

/**
* Updates the runtime control buttons based on the program state
* @param {boolean} isRunning - Whether the program is currently running
*/
function updateRuntimeButtons(isRunning) {
toggleButtonState("stopProgram", isRunning);
toggleButtonState("collapsedStopProgram", isRunning);
Comment on lines +317 to +374
Copy link
Contributor

Choose a reason for hiding this comment

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

This should all be in editorMain.js - there's a lot of framework code already there to help. Have a look in the functions setupIDEButtonEvents and updateButtons 😃

}

/**
* Toggles the disabled state of a button
* @param {string} buttonId - The ID of the button to toggle
* @param {boolean} isEnabled - Whether the button should be enabled
*/
function toggleButtonState(buttonId, isEnabled) {
const button = document.getElementById(buttonId);
if (button) {
button.disabled = !isEnabled;
}
}

/**
* Checks if a program is currently running
* @returns {boolean} - True if a program is running, false otherwise
*/
function isProgramRunning() {
// Replace this with the actual logic to check program state
return runtimeManager.isRunning();
}

/**
* Displays a notification to the user
* @param {string} message - The message to display
* @param {string} type - The type of the notification ("info", "warning", "error")
*/
function showNotification(message, type) {
// Replace this with your actual notification logic
console.log(`[${type.toUpperCase()}] ${message}`);
Comment on lines +388 to +405
Copy link
Contributor

Choose a reason for hiding this comment

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

These don't make sense, I'm assuming AI generated?

}
8 changes: 8 additions & 0 deletions Browser_IDE/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ <h1>Let's write some code!</h1>
<i class="bi bi-gear-fill"></i>
<i class="bi bi-play-fill">Build and Run</i>
</button>
<button type="button" id="stopProgram">
<i class="bi bi-stop-fill"></i>
<span>Stop</span>
</button>
<button type="button" id="collapsedStopProgram" title="Stop Program">
<i class="bi bi-stop-fill">Terminate</i>
</button>

<button type="button" id="stopProgram" style="display:none"><i class="bi bi-pause-fill">Pause</i></button>
Comment on lines +103 to 111
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is working as intended - we end up with two stop buttons by accident, and the pause button is hidden since it has the same ID stopProgram:
image
The collapsedStopProgram button should only be visible when the interface is collapsed, and the normal pause button should still be visible.
Maybe something like this?
image

Copy link
Author

Choose a reason for hiding this comment

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

thank you for the feedback, I will change this.

</div>
<div class="sk-header-title">
Expand Down
Loading