#1284 On Win32, remove frame from auto-completion box

[Zufu Liu]

This change make the auto-completion box looks the same on all system.
On Win10 it has 6px white frame above the window. Change WS_THICKFRAME to WS_BORDER could remove it.
Tested other apps doesn't using window frame: Visual Studio 2017/2019, CodeLite.

Drawbacks compared to WS_THICKFRAME: resizing by mouse is harder, ListBoxX::NcHitTest() needs improvement.

Some links
https://stackoverflow.com/questions/39731497/create-window-without-titlebar-with-resizable-border-and-without-bogus-6px-whit
https://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/6238e230-5b64-4f64-871a-b21b9b668df1/windows-10-nonclient-area

[Neil Hodgson]

Resizing should be active over more than 1 pixel. Best would be adding another pixel or 2 of border but that may be difficult to make look good. Its likely OK to steal a pixel or 2 from the interior bottom/right of the list box for resizing.

I personally prefer the old Win7 thick frame showing that resizing is possible but Win10 should be the focus for appearance and functionality. Win7 should continue to work but it can be the loser if there is conflict for appearance.

Inflate and Deflate only differ by sign so a single Inflate method is sufficient with YAGNI applying here:

constexpr PRectangle Inflate(XYPOSITION xDelta, XYPOSITION yDelta) const noexcept

[Zufu Liu]

Attached patch draw a 4px transparent frame, resizing is easier, except the code inside WM_NCPAINT has bug on resizing.

With WS_BORDER, (on Win10 with default 1px border) I only get HTBORDER from default WM_NCHITTEST.

[Neil Hodgson]

Yes, the resize drawing is poor. The area where resize is active appears OK although there is only ever horizontal or vertical resize cursor, never diagonal.

[Zufu Liu]

A rough implementation for diagonal:

/*
13 | 12 | 14         4 | 3 | 5
10 |    | 11    =>   1 | 0 | 2
16 | 15 | 17         7 | 6 | 8
*/
const int x = (xPos <= rcInner.left) ? 1 : ((xPos >= rcInner.right) ? 2 : 0);
const int y = (yPos <= rcInner.top) ? 3 : ((yPos >= rcInner.bottom) ? 6 : 0);
const int h = x + y;
hit = h ? (10 + h - 1) : HTERROR;

[Zufu Liu]

full code for hit test:

else if (hit < HTSIZEFIRST || hit > HTSIZELAST) {
    const int cx = GetSystemMetrics(SM_CXVSCROLL);
    const PRectangle rcInner = rc.Deflate(ListBoxXFakeFrameSize, ListBoxXFakeFrameSize);
    const int xPos = GET_X_LPARAM(lParam);
    const int yPos = GET_Y_LPARAM(lParam);
    /*
    13 | 12 | 14         4 | 3 | 5
    10 |    | 11    =>   1 | 0 | 2
    16 | 15 | 17         7 | 6 | 8
    */
    const int x = (xPos <= rcInner.left) ? 1 : ((xPos >= rcInner.right - cx) ? 2 : 0);
    int y = (yPos <= rcInner.top) ? 3 : ((yPos >= rcInner.bottom) ? 6 : 0);
    if (y == 0 && x == 2) {
        if (location.y < rc.top) {
            y = (yPos >= rcInner.bottom - cx) ? 6 : 0;
        } else {
            y = (yPos <= rcInner.top + cx) ? 3 : 0;
        }
    }
    const int h = x + y;
    hit = h ? (9 + h) : HTERROR;
}

[Neil Hodgson]

That behaves better.

Using arithmetic and magic constants obscures the intent.

[Jad Altahan]

If you are going to commit this change, please consider making to execlusive to Windows 10. Removing WS_THICKFRAME on Windows 7 makes the the auto-completion box look really unpolished, imo. This flat look doesn't go well the the general UI design of pre-Win 8.

[Neil Hodgson]

There is a potential change to make resizing an option at https://www.scintilla.org/AutocompletionNonResizing.patch .

[Neil Hodgson]

This needs the order to be changed to work first time: ac.lb->SetOptions should be called before ac.Start.

[Zufu Liu]

ListOptions options can be passed into ac.Start(), then calls lb->SetOptions() before lb->Create() inside AutoComplete::Start().

[Zufu Liu]

Update comment for ListBoxX::NcHitTest() for WS_BORDER, the code is similar to https://sourceforge.net/p/scintilla/feature-requests/1284/#5617

/*
HTTOPLEFT    13 | HTTOP    12 | HTTOPRIGHT    14         4 | 3 | 5
HTLEFT       10 | HTNOWHERE 0 | HTRIGHT       11  -9 =>  1 | 0 | 2
HTBOTTOMLEFT 16 | HTBOTTOM 15 | HTBOTTOMRIGHT 17         7 | 6 | 8
*/

[Neil Hodgson]

This, on top of AutocompletionNonResizing.patch still makes resizing very difficult. Trying to graft in ac-frame2.diff doesn't help. Perhaps there is some difference in our systems (DPI?) that make this work OK for you.

It is dark mode implementers that motivated the proposed change: the white top bar looks particularly bad in dark mode.

AutocompletionNonResizing.patch is saying: either you get resizing and an extra white bar or you get a fixed size and no extra bar. That's why the option to get WS_BORDER is called SC_AUTOCOMPLETE_FIXED_SIZE.

[Zufu Liu]

Yeah, resizing is harder for WS_BORDER even with NcHitTest() changes.
Since application can use AutoCSetMaxHeight() and AutoCSetMaxWidth() to set auto-completion box to a prefect size before calling AutoCShow(), I think this is not important.

Following is the code used by Notepad2:
https://github.com/zufuliu/notepad2/blob/master/src/EditAutoC.c#L1453

SciCall_AutoCSetMaxWidth(pWList->iMaxLength << 1); // width columns, default auto
SciCall_AutoCSetMaxHeight(min_u(pWList->nWordCount, autoCompletionConfig.iVisibleItemCount)); // visible rows

There is one confusing thing: AutoCSetMaxWidth() does not works as expected without multiply max word length (in bytes) by a factor large than 1, otherwise longest item will be drawn with trailing ellipsis.

LaTeXInput-4.diff at https://sourceforge.net/p/scintilla/feature-requests/1407/#688a/d0b9/1b17 has similar changes for SciTE.

[Neil Hodgson]

LaTeXInput-4.diff

There were problems with this and I undid some of the changes. There shouldn't be a need to call AutoCSetMaxHeight with an expression: just leave it at the preference value since it adjusts for the list length.

AutoCSetMaxWidth is based on average character width. Sizing for widest item just picks the longest string and measures that instead of measuring each item, so "iiiii" would determine the width instead of "WWWW". It would probably be reasonable to measure each item now. The current technique was likely trying to avoid slowness back in ancient times.

[Neil Hodgson]

Committed the non-resizing option.