replit / desktop

Replit Desktop App
114 stars 3 forks source link

Remove "close" menu item #124

Closed sergeichestakov closed 1 year ago

sergeichestakov commented 1 year ago

Why

Perhaps this is due to the recent Electron upgrade (although I can't find anything in the Changelog about this) but hitting Cmd+W on Mac now seems to close the window in the app. This is undesirable because we want to leave this keyboard shortcut available for users to set and also want to prevent that shortcut from closing the window as it does not in most other apps / IDEs. Turns out it's because we add a menu item with the "close" role which sets this keyboard shortcut. (see relevant SO post and GH issue)

Additionally, I was thinking we should remove the "Open Repl URL from Clipboard" menu item. It's too niche to be useful (since you have to have a Repl URL copied in your clipboard to begin with) and is hard to find / not intuitive anyways.

Fixes WS-965

What changed

Remove "Close" and "Open Repl URL from clipboard" menu items

Test plan

linear[bot] commented 1 year ago
WS-965 Not sure if this bug is tracked but wanted to flag. On macOS when I hit command-w with a web view...

dh said: > Not sure if this bug is tracked but wanted to flag. On macOS when I hit command-w with a web view running, it closes the entire app (like the behavior on browser). This doesn't seem to happen on the web view when you haven't run the Repl and only when it's running.

sergeichestakov commented 1 year ago

hmm what's the benefit of keeping close around though even for just the home page? I think we should have consistent behavior across both the home page and workspace. It feels unexpected to me to use Ctrl/Cmd+W to close tabs in the WS but have it close the entire app window in the home page or if I unset that keyboard shortcut preference.

As for the open from clipboard menu item, I don't feel that strongly and really undiscoverable / unintuitive (we only know how it works because we built it) but I do think it's bad UX and we can solve this problem in a better way via the web UI

szymonkaliski commented 1 year ago

hmm what's the benefit of keeping close around though even for just the home page?

Well, I open the app, see the home page, want to close it -> cmd-w doesn't work = it feels broken.

It feels unexpected to me to use Ctrl/Cmd+W to close tabs in the WS but have it close the entire app window in the home page or if I unset that keyboard shortcut preference.

I don't agree. Look at Safari for example: cmd-w closes tabs, unless you're on the last tab, then it closes the window. I wouldn't say it's inconsistent. If you open preferences window in the same Safari, and hit cmd-w it will close the preferences window. That's exactly what we have right now, if you have that keybinding set -- and I'd argue the solution here is to have "close tab" always set to cmd-w in the desktop app, or at least set by to cmd-w by default.

sergeichestakov commented 1 year ago

I'm not sure why we'd need to copy Safari though. One of the primary benefits of the desktop app is that we don't have to deal with conflicts with the browsers keyboard shortcuts and I'm not convinced that this is useful enough to warrant keeping around

szymonkaliski commented 1 year ago

I gave Safari as an example of a native mac desktop application with tabs to show the behavior of cmd-w. Again, all mac applications use cmd-w to close the tab, and then the window, and to close the window if it has no tabs. Breaking this is a very bad idea.

Do you have any examples of native mac applications that don't respect cmd-w?

sergeichestakov commented 1 year ago

Hmm fair enough. We can keep it but that still begs the question why the behavior is so inconsistent especially in server Repls. You can see I do have Cmd+W set in the Workspace but it still closes the window unless certain panes are open. Perhaps it has something to do with panes being out of focus.

See demo (every time the window is closed it's due to Cmd+W):

inconsistent-cmd-w

sergeichestakov commented 1 year ago

closing for now but we still need to fix the above issue. any chance you could take a look?