-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Add Status bar Indicator for Overwrite mode #6670
Add Status bar Indicator for Overwrite mode #6670
Conversation
@WebsiteDeveloper could you attach a screenshot? Don't have a laptop with me right now. |
|
I'm wondering if we could just make it toggle between "insert" and "overtype" like the tab/space toggle without the green background. Overtype mode is pretty obvious and requires user activation first so we probably won't need to call it out like that. If for some reason a background color is needed then we should use #78B2F2 (blue) for the active state, keeping the gray border on both sides. Thoughts? |
@larz0, the original bug was about people accidentally hitting the Ins key on Windows and put Brackets into overwrite mode without knowing they did it. So, while toggling the mode via the status bar is more obvious, toggling the mode by accidentally hitting the Ins key is not obvious at all. I'm not sure a green background is the best way to do it, but something should be done to bring the toggled state to the user's attention. |
Personally I don't mind calling it out strongly, but I think there's also a reasonable argument for not doing that: in every other editor the statusbar indication is quite subtle, and the cursor appearance change is a more direct indicator (in context) anyway. |
@lkcampbell I see. So either the behavior of the cursor input isn't obvious or the user don't know what activated overtype mode. (Is this common on Windows?) If that's the case then let's use the blue background on the indicator. |
@larz0, actually, @peterflynn has a good point. I didn't think about the cursor change. That is a good visual indicator. |
I think it might look good to just change the color of the text "OVW" from a light gray (deactivated) to black (activated). |
@lkcampbell ok cool. @TomMalbran let's change the background color if it's absolutely as it's a little heavy-handed, what @peterflynn mentioned above. |
@TomMalbran we could do that so it's more obvious for the Mac users that overtype feature is available. |
I think changing the background color might be too much. Visual Studio displays "INS" in insert mode and "OVR" in overwrite mode. They also change the cursor. |
Is there some kind of consensus what to do? |
Notepad++ does the exact same thing. Looking back at the issue this PR fixes (#6206), they are wanting some visual feedback on what mode is enabled (as most are suggested be done). #4250, on the other hand, suggested the same thing @redmunds is. Personally, I think either solution would work and solve the issue here. A small area at the bottom of your screen suddenly turning blue would catch pretty much anybody's eye, and it gives near-instant feedback on what mode is activated. Simply changing the cursor to an
doesn't provide too much feedback, and it can be hard for some people to discern between the two semi-subtle color changes (like myself. I am slightly color-blind, and the black/dark gray area is one of trouble spots). You could say the answer is summed up in one question: do you want to do the same as everyone else, or do you want to be different and roll your own way? |
Another idea: give the indicator a background-color that fades away after a few seconds. That way, you'd notice when it changes, but it wouldn't always stand out. |
Like @le717 said, traditionally, the abbreviations I have seen in editors are INS and OVR. The toggle flips the abbreviations. I suspect this would be something that might need to be translated into appropriate abbreviations in other languages as well. I've never seen OVW used before. Is that from an editor I haven't seen before? The cursor shape change is a really good indicator and I also really like @redmunds idea as well, a flash of background color that fades to catch the user's attention. |
@WebsiteDeveloper We need to wait for @larz0 to make a decision. Note that he's on vacation so may be slow to respond. FYI, I'm waiting for you to respond to my comments on related pull request #6328 if you want to look at that one. |
@redmunds @lkcampbell A quick flash of color/possibly label change plus the cursor sounds like the best of both ideas. You get the sudden visual indication and a more subtle constant reminder. You'd just have to make sure the blue background stayed up long enough for the user to understand what it means. |
... But if that is chosen, then an idea for switching back to insert mode needs to be devised, probably along the same lines but using a different color/label change. |
@lkcampbell +1 for INS/OVR toggle. We could use white text on #78B2F2 for the flash. |
@WebsiteDeveloper The INS indicator doesn't change for me. I expected it to change when I press "Insert" key on Windows or if I click on it in Statusbar. What do you think about combining both Insert Mode pull requests (this one and #6328) into a single branch and pull request so we can see how it all works together? |
@redmunds You need codemirror/codemirror5@f9aa052 to try this PR, I had to do the same. @WebsiteDeveloper I don't know if @larz0 meant to flash the indicator on every toggle, I think it should not stay blue. |
@larz0 Quick screencast: In OVR mode, it stays blue until you toggle into INS mode. |
@SAplayer thanks for the screencast. Looks good! |
The blue should also fade out in overtype mode. |
Just another screencast for @larz0: |
I think switching the text color between black and white during highlighting makes it hard to read. I think text should always be black. If that makes it hard to read against dark background, then maybe background shouldn't be as dark, or text should have a light text-shadow. |
Ok let's keep the text black to keep it simple. |
@WebsiteDeveloper I think you could also use |
function _updateEditorOverwriteMode() { | ||
var editor = EditorManager.getActiveEditor(); | ||
|
||
editor._codeMirror.toggleOverwrite(null); |
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.
Should we make a new Editor.toggleOverwrite
method which just does this._codeMirror.toggleOverwrite(value)
, so that we don't access the private member _codeMirror
?
@SAplayer i didn't know that module existed^ |
@@ -921,6 +924,15 @@ define(function (require, exports, module) { | |||
}; | |||
|
|||
/** | |||
* Sets or toggles the editors overwrite mode. | |||
* | |||
* @param {!boolean} start |
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.
Should be @param {?boolean} state
since you can pass null too. Might be nice to have a description that you can use a boolean value to set and null to toggle the state.
Also, you could the empty line.
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.
oh forgot that you're right
Fwiw, a screencast for @larz0 again: |
@WebsiteDeveloper Should we maybe rename the class |
Looks good! Thanks! |
This pull is superseded by #6777 |
@WebsiteDeveloper Please close this PR if if is superseded by #6777 |
oh forgot that |
@larz0 for UI review.
Please notice the ability to indicate the current toggle state when changed via insert key is depending on codemirror/codemirror5@f9aa052 to be include. But it can be changed via clicking on the status bar item.