status bar #432
Open
status bar #432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krashanoff
reviewed
Feb 5, 2021
There was a problem hiding this comment.
Prateek, thanks for getting to work on this! Component itself is real simple, but I like the idea -- it's a great start. Right now, my comments are:
- The user should be able to toggle the status line while editing. The status line should be off by default.
- The status line should be toggled somewhere. I'm thinking we add a button or remove it for the time being? It will be integrated into the collaborative editor down the line.
- To that end, we will probably want to develop this feature on its own branch, and make pull requests against that. I will discuss this a little more in our meeting tonight.
To elaborate on the logic of why I suggested using the CodeMirror programming API in #430, I think that it would keep our styles a little tighter, since the status line would then update with light/dark mode and generally be easier to maintain. I would like your input on this if you feel a certain way, but I am focused on keeping the editor maintainable where possible.
krashanoff
suggested changes
Feb 21, 2021
There was a problem hiding this comment.
Hey Prateek, sorry for the delay on getting this review out. I have a few small changes. Many are nitpicky. The only significant one is that I think we should mask the toggle button from the editor for the moment, as this feature won't be useful outside of the collaborative environment. So, for now, we will have the status line ready to use when we cross that bridge.
Let me know if anything in my review is unclear. Otherwise, I'll keep an eye out for the next round of changes.
.eslintcache
Outdated
| @@ -0,0 +1 @@ | |||
| [{"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\index.js":"1","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\containers\\AppContainer.js":"2","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\index.js":"3","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\containers\\MainContainer.js":"4","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\app.js":"5","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\programsReducer.js":"6","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Login.js":"7","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\EditorAndOutput\\EditorAndOutput.js":"8","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\index.js":"9","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\containers\\EditSketchModalContainer.js":"10","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\OpenPanelButton.js":"11","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\components\\DropdownButton.js":"12","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\lib\\sketch.js":"13","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\components\\StatusBar.js":"14","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\components\\TextEditor.js":"15","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\ProfilePanel.js":"16","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\uiReducer.js":"17","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\actions\\uiActions.js":"18","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\containers\\ProfilePanelContainer.js":"19","D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\containers\\TextEditorContainer.js":"20"},{"size":764,"mtime":1612404807718,"results":"21","hashOfConfig":"22"},{"size":1623,"mtime":1602724010484,"results":"23","hashOfConfig":"22"},{"size":407,"mtime":1602724010893,"results":"24","hashOfConfig":"22"},{"size":2155,"mtime":1612404807696,"results":"25","hashOfConfig":"22"},{"size":6586,"mtime":1612404807625,"results":"26","hashOfConfig":"22"},{"size":1268,"mtime":1602724010900,"results":"27","hashOfConfig":"22"},{"size":4931,"mtime":1602724006777,"results":"28","hashOfConfig":"22"},{"size":3621,"mtime":1602724006775,"results":"29","hashOfConfig":"22"},{"size":5955,"mtime":1612404807589,"results":"30","hashOfConfig":"22"},{"size":801,"mtime":1602724009096,"results":"31","hashOfConfig":"22"},{"size":789,"mtime":1602724010456,"results":"32","hashOfConfig":"22"},{"size":2280,"mtime":1602724008820,"results":"33","hashOfConfig":"22"},{"size":738,"mtime":1602724010785,"results":"34","hashOfConfig":"22"},{"size":482,"mtime":1613086981399,"results":"35","hashOfConfig":"22"},{"size":11539,"mtime":1613087017437,"results":"36","hashOfConfig":"22"},{"size":10474,"mtime":1613088242026,"results":"37","hashOfConfig":"22"},{"size":1003,"mtime":1613086930261,"results":"38","hashOfConfig":"22"},{"size":634,"mtime":1613086901589,"results":"39","hashOfConfig":"22"},{"size":1259,"mtime":1613087549546,"results":"40","hashOfConfig":"22"},{"size":1564,"mtime":1613087652148,"results":"41","hashOfConfig":"22"},{"filePath":"42","messages":"43","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},"1xdarae",{"filePath":"44","messages":"45","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"46","messages":"47","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"48","messages":"49","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"50","messages":"51","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"52","messages":"53","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"54","messages":"55","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"56","messages":"57","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"58","messages":"59","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"60","messages":"61","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"62","messages":"63","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"64","messages":"65","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"66","messages":"67","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"68","messages":"69","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"70","messages":"71","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"72","messages":"73","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"74","messages":"75","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"76","messages":"77","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"78","messages":"79","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},{"filePath":"80","messages":"81","errorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0},"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\index.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\containers\\AppContainer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\index.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\containers\\MainContainer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\app.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\programsReducer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Login.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\EditorAndOutput\\EditorAndOutput.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\index.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\containers\\EditSketchModalContainer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\OpenPanelButton.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\Sketches\\components\\DropdownButton.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\lib\\sketch.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\components\\StatusBar.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\components\\TextEditor.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\ProfilePanel.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\reducers\\uiReducer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\actions\\uiActions.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\common\\containers\\ProfilePanelContainer.js",[],"D:\\Projects\\TeachLA\\TeachLAFrontend\\src\\components\\TextEditor\\containers\\TextEditorContainer.js",[]] | |||
There was a problem hiding this comment.
This file can be safely deleted. I will add it to our .gitignore a little later.
| return ( | ||
| <div className="status-bar-container"> | ||
| <p className="status-bar-info"> | ||
| {props.editing ? "Editing" : "Viewing"} line: {props.line + 1} col: {props.col + 1}{" "} | ||
| <p className="status-bar-info" style={textStyle}> |
There was a problem hiding this comment.
I still think that we could integrate this with our built-in styles with something like this instead:
<p className="status-bar-info" className={props.theme === "light" ? "light-status" : "dark-status"}>
| @@ -316,6 +316,19 @@ class ProfilePanel extends React.Component { | |||
| </div> | |||
| ); | |||
|
|
|||
| renderShowStatusBar = () => { | |||
| let onToggle = () => { | |||
| console.log(this.props); | |||
There was a problem hiding this comment.
We can remove extraneous logging.
Suggested change
| console.log(this.props); |
Comment on lines
324
to
331
| return ( | ||
| <div className="panel-button"> | ||
| <button className="statusbar-button" onClick={() => onToggle()}> | ||
| <span> Status Bar</span> | ||
| </button> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
For now, let's mask the button from the editor, then we can add a button for it when we create the collaborative editor. Apologies if that wasn't clear. As a side note, using the same classes as themeSwitch, etc. would benefit its uniformity:
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
created basic status bar. Displays whether editing or viewing. Shows line and column number. adds option to choose to display bar or not