windows: restore window position #123

Merged
merged 9 commits into from Oct 12, 2012

Projects

None yet

2 participants

@redmunds

Glenn submitted a fix for this on Mac, so now we can take this fix for Windows.

@gruehle gruehle and 1 other commented on an outdated diff Oct 3, 2012
appshell/cefclient_win.cpp
@@ -55,6 +55,22 @@
#pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"") // NOLINT(whitespace/line_length)
#endif
+// Registry access functions
+bool GetRegistryInt(LPCWSTR pFolder, LPCWSTR pEntry, int* pDefault, int& ret);
+bool WriteRegistryInt (LPCWSTR pFolder, LPCWSTR pEntry, int val);
+
+// Registry key strings
+#define PREF_BRACKETS_BASE L"Software\\Brackets\\"
@gruehle
gruehle Oct 3, 2012

This should use the GROUP_NAME and APP_NAME constants defined in config.h instead of hard-coding "Brackets"

@gruehle
gruehle Oct 3, 2012

Oh, and PREF_BRACKETS_BASE should be given a more generic name like PREF_APPSHELL_BASE, too.

@gruehle gruehle was assigned Oct 3, 2012
@redmunds

Code review changes pushed.

@redmunds redmunds referenced this pull request in adobe/brackets Oct 9, 2012
Closed

Brackets Size not remembered when closing #1594

@gruehle gruehle and 1 other commented on an outdated diff Oct 10, 2012
appshell/cefclient_win.cpp
+ ASSERT(pBase && pApp && pRet);
+ if (!pBase || !pApp || !pRet)
+ return;
+
+ // Base
+ wcscpy(pRet, pBase);
+
+ // Group (optional)
+ if (pGroup && (pGroup[0] != '\0'))
+ {
+ wcscat(pRet, L"\\");
+ wcscat(pRet, pGroup);
+ }
+
+ // App name
+ wcscat(pRet, L"\\");
@gruehle
gruehle Oct 10, 2012

the GROUP_NAME macro, if defined, already has a backslash at the end of it (see config.h for edge code). In that case, you'll either need to omit the backslash when copying from pGroup, or omit it here before appending pApp.

@redmunds
redmunds Oct 11, 2012

Fixed. I made it so any string passed to this function works with or without a trailing backslash.

@gruehle gruehle and 1 other commented on an outdated diff Oct 10, 2012
appshell/cefclient_win.cpp
@@ -247,9 +377,17 @@ BOOL InitInstance(HINSTANCE hInstance, int nCmdShow) {
hInst = hInstance; // Store instance handle in our global variable
- hWnd = CreateWindow(szWindowClass, szTitle,
- WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN, CW_USEDEFAULT, 0,
- CW_USEDEFAULT, 0, NULL, NULL, hInstance, NULL);
+ // TODO: test this cases:
+ // - window in secondary monitor when shutdown, disconnect secondary monitor, restart
+
+ int left = CW_USEDEFAULT;
+ int top = 0;
+ int width = CW_USEDEFAULT;
+ int height = 0;
+ RestoreWindowRect(left, top, width, height);
@gruehle
gruehle Oct 10, 2012

This will restore the window rect, but doesn't restore the maximized flag or the "normal" size. If you maximize the window, quit, then restart, the window size is set to the maximized size, but it isn't maximized (and, at least on my Win 7 machine, the top of the titlebar is a bit off the top of the screen). You may need to use GetWindowPlacement/SetWindowPlacement and save/restore the whole WINDOWPLACEMENT structure.

@redmunds
redmunds Oct 11, 2012

Fixed. This is how window states should be restored:

restored - Size & position should be remembered on any monitor.

maximized - Should be remembered for any monitor. Also, switching back to "restored" size should remember previous size & position.

minimized - It seems like we didn't want to open Brackets into a minimized state, so it should use state from previous startup.

@gruehle
Adobe Systems Incorporated member

Initial review complete.

@redmunds

@gruehle Changes pushed.

@gruehle
Adobe Systems Incorporated member

Looks good. Merging.

@gruehle gruehle merged commit 4bc4bb4 into master Oct 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment