-
-
Notifications
You must be signed in to change notification settings - Fork 61
FIX ISSUE #204 : Add Essential Keyboard Shortcuts for Better User Experience #205
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
base: main
Are you sure you want to change the base?
Conversation
|
@Stonebanks-js is attempting to deploy a commit to the Dhanush Nehru's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@DhanushNehru The Liniting check is fixed and passed successfully, I hereby request you to check the deployment status. |
|
@DhanushNehru Kindly review this PR and add hacktoberfest labels please |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds keyboard/UX enhancements and refactors the editor page, with improved Judge0 submission handling and import/export utilities. However, the advertised global keyboard shortcuts are not implemented in the diff, and there appear to be regressions in the sidebar controls.
- Adds submit/polling improvements and safer theme definition.
- Implements import/export helpers, output copy/clear, and state cleanup on unmount.
- Wires a global keydown handler for Ctrl+Enter only.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/pages/EditorComponent.js | Refactors editor logic, adds import/export and output utilities, adds Ctrl+Enter global handler, adjusts state handling and UI blocks. |
| package.json | Bumps eslint to ^8.57.1. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@DhanushNehru Sir please merge this PR, all the checks have passed successfully, Kindly assign it with hacktoberfest labels 2025. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const isImportingRef = useRef(false); | ||
| const fileInputRef = useRef(null); | ||
| const timeoutRef = useRef(null); | ||
| const [isDownloading, setDownloading] = useState(false); |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isDownloading state is declared but never set to true anywhere in the code. The setDownloading(true) call was removed from the exportFile function, making this state variable unused. Consider removing this state if it's not needed, or ensure it's properly managed if the downloading indicator is still required.
| const [isDownloading, setDownloading] = useState(false); |
| const decoded = atob(result.stdout); | ||
| setOutput(decoded.split("\n")); | ||
| } else if (result && result.compile_output) { | ||
| const decoded = atob(result.compile_output); | ||
| setOutput(decoded.split("\n")); | ||
| enqueueSnackbar("Compilation error. See output.", { variant: "error" }); | ||
| } else if (result && result.stderr) { | ||
| const decoded = atob(result.stderr); | ||
| setOutput(decoded.split("\n")); |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The decodeFormat function is defined earlier in the file but is no longer being used. The base64 decoding and splitting logic has been duplicated inline here and in multiple other places (lines 236, 240). Consider using the existing decodeFormat function to maintain consistency and reduce code duplication.
| const decoded = atob(result.stdout); | |
| setOutput(decoded.split("\n")); | |
| } else if (result && result.compile_output) { | |
| const decoded = atob(result.compile_output); | |
| setOutput(decoded.split("\n")); | |
| enqueueSnackbar("Compilation error. See output.", { variant: "error" }); | |
| } else if (result && result.stderr) { | |
| const decoded = atob(result.stderr); | |
| setOutput(decoded.split("\n")); | |
| setOutput(decodeFormat(result.stdout)); | |
| } else if (result && result.compile_output) { | |
| setOutput(decodeFormat(result.compile_output)); | |
| enqueueSnackbar("Compilation error. See output.", { variant: "error" }); | |
| } else if (result && result.stderr) { | |
| setOutput(decodeFormat(result.stderr)); |
| java: "Java", | ||
| }; | ||
|
|
||
| const languageMap = { js: "Javascript", jsx: "Javascript", ts: "Typescript", py: "Python3", cpp: "C++", c: "C", java: "Java" }; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This language mapping is now defined inline within the handleFileImport function, but a similar mapping exists in the exportFile function at line 335. Consider extracting this to a constant at the module level to maintain consistency and avoid duplication.
| <Box sx={{ flexGrow: 1 }}> | ||
| {currentUser ? renderAuthenticatedContent() : ( | ||
| <div> | ||
| <GithubSignIn /> | ||
| <GoogleSignIn /> | ||
| </div> | ||
| </Box> | ||
| {currentUser | ||
| ? renderAuthenticatedContent() | ||
| : renderUnauthenticatedContent()} | ||
| <div className="footer"> | ||
| <Footer /> | ||
| </div> | ||
| </div> | ||
| )} | ||
| <Footer /> | ||
| </Box> |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire header section with logo, theme toggle, stars component, and user profile has been removed from the render. This removes critical UI elements including the logout button and user information display, which breaks the user experience. The handleSignOut function is still defined but no longer accessible from the UI.
| {/* Run, Copy, Clear */} | ||
| <div style={{ display: "flex", gap: "0.5rem", marginTop: "1rem" }}> | ||
| <StyledButton onClick={submitCode}><FaPlay /> Run</StyledButton> | ||
| <StyledButton onClick={copyOutput}><FaCopy /> Copy Output</StyledButton> | ||
| <StyledButton onClick={clearOutput}><FaTrash /> Clear Output</StyledButton> | ||
| </div> | ||
| <StyledButton | ||
| sx={(theme) => ({ | ||
| marginTop: "1rem", | ||
| padding: "10px 20px", | ||
| bgcolor: theme.palette.text.primary, | ||
| color: theme.palette.background.default, | ||
| border: "none", | ||
| borderRadius: "15px", | ||
| fontSize: "0.8em", | ||
| cursor: "pointer", | ||
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | ||
| width: "50%", | ||
| "@media (min-width: 1024px)": { | ||
| width: "100%", | ||
| }, | ||
| })} | ||
| onClick={submitCode} | ||
| variant="contained" | ||
| color="primary" | ||
| disabled={loading} | ||
| > | ||
| <span> | ||
| {loading ? <CircularProgress size={13} /> : <FaPlay size="13" />} | ||
| </span> | ||
| Run {languageDetails.LANGUAGE_NAME} | ||
| </StyledButton> | ||
| {/* Output */} | ||
| <OutputLayout> | ||
| <pre>{output && output.length ? output.join("\n") : "Output will appear here..."}</pre> | ||
| </OutputLayout> |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The language selector, theme selector, and editor settings (line numbers toggle, word wrap toggle, font size slider) have been completely removed from the UI. While these state variables and handlers still exist in the code, users can no longer access these controls. This breaks functionality that was previously available.
| Export | ||
| </> | ||
| )} | ||
| <input type="file" ref={fileInputRef} style={{ display: "none" }} accept=".java,.js,.jsx,.ts,.py,.cpp,.c" onChange={handleFileImport} /> |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description mentions adding keyboard shortcuts including Ctrl+O for opening/importing files, but there is no keyboard event listener implemented for this functionality. The file input only works via button click, not via the Ctrl+O shortcut as described.
|
The landing page is not proper @Stonebanks-js
|

⌨️ Add Keyboard Shortcuts Feature
ISSUE FIXED : #204
Changes:
Editor.jsxto support:clearOutputandcopyOutputpropsTests / Manual Verification:
Please review and test if it causes any conflicts with Monaco internal keybindings.
Closes issue #204
@DhanushNehru I request you to run checks and merge the pull request into main.