Re: comctl32: SysDateTimePick32 control add Alt+down hot key likewindows (try 2)

[Nikolay Sivov]

On 25.02.2015 10:56, Changhui Liu wrote:
> Superseded patch 109085
> http://source.winehq.org/patches/data/109085
>
> Change log:
> 1, Pass test on all Windows.
>
> 2, Add DTN_DROPDOWN and DTN_CLOSEUPnotification test.
>
> 3,
> > > /  +    case WM_SYSKEYDOWN:
> > > />>/  +        if (wParam == VK_DOWN)
> > > />>/  +        {
> > > />>/  +            POINT pt;
> > > />>/  +            RECT rect;
> > > />>/  +            GetClientRect(hwnd, &rect);
> > > />>/  +            pt.x = rect.right - GetSystemMetrics(SM_CXVSCROLL)/2;
> > > />>/  +            pt.y = rect.top + GetSystemMetrics(SM_CYHSCROLL)/2;
> > > />>/  +
> > > />>/  +            return DATETIME_LButtonDown (infoPtr, (SHORT)pt.x, (SHORT)pt.y);
> > > />>/  +        }
> > > />>/  +
> > > />>
> > > If it really works this way you should just make it drop directly,
> > > without faking click point coordinates.
>
> Because most of code of the directly implement are same as DATETIME_LButtonDown,
> and too many repeat code looks bad.
> So faking click point coordinates can reuse code.

I'm not saying you should duplicate it, but reuse it in a clean way 
without hit testing - move relevant code somewhere and use it.
> +    if (WM_NOTIFY == message)
> +    {
> +        NMHDR* hdr = (NMHDR*)lParam;
> +
> +        if (hdr->code == DTN_DROPDOWN)
> +        {
> +            g_hWndMonthCal = (HWND)SendMessageA(hdr->hwndFrom, DTM_GETMONTHCAL, 0, 0);
> +            trace("open SysMonthCal32=%p\n", g_hWndMonthCal );
> +        }
> +        else if (hdr->code == DTN_CLOSEUP)
> +        {
> +            trace("close SysMonthCal32=%p\n", g_hWndMonthCal);
> +        }
> +    }

That's not a notification test as you don't check anything. Please add a 
proper message sequence test, we have tons of examples for those.

comctl32: SysDateTimePick32 control add Alt+down hot key likewindows(try 2)

[Changhui Liu]

Hi Nikolay,
I update a new patch, do you think is it OK now ?
Thank you.






------------------
Regards,
Changhui.
 

 
 
 
------------------ Original ------------------
From:  "Nikolay Sivov"<bunglehead(a)gmail.com>;
Date:  Wed, Feb 25, 2015 04:12 PM
To:  "Changhui Liu"<liuchanghui(a)linuxdeepin.com>; 
Cc:  "wine-devel"<wine-devel(a)winehq.org>; 
Subject:  Re: comctl32: SysDateTimePick32 control add Alt+down hot key likewindows(try 2)

 
On 25.02.2015 10:56, Changhui Liu wrote:
> Superseded patch 109085
> http://source.winehq.org/patches/data/109085
>
> Change log:
> 1, Pass test on all Windows.
>
> 2, Add DTN_DROPDOWN and DTN_CLOSEUPnotification test.
>
> 3,
> > > /  +    case WM_SYSKEYDOWN:
> > > />>/  +        if (wParam == VK_DOWN)
> > > />>/  +        {
> > > />>/  +            POINT pt;
> > > />>/  +            RECT rect;
> > > />>/  +            GetClientRect(hwnd, &rect);
> > > />>/  +            pt.x = rect.right - GetSystemMetrics(SM_CXVSCROLL)/2;
> > > />>/  +            pt.y = rect.top + GetSystemMetrics(SM_CYHSCROLL)/2;
> > > />>/  +
> > > />>/  +            return DATETIME_LButtonDown (infoPtr, (SHORT)pt.x, (SHORT)pt.y);
> > > />>/  +        }
> > > />>/  +
> > > />>
> > > If it really works this way you should just make it drop directly,
> > > without faking click point coordinates.
>
> Because most of code of the directly implement are same as DATETIME_LButtonDown,
> and too many repeat code looks bad.
> So faking click point coordinates can reuse code.

I'm not saying you should duplicate it, but reuse it in a clean way 
without hit testing - move relevant code somewhere and use it.
> +    if (WM_NOTIFY == message)
> +    {
> +        NMHDR* hdr = (NMHDR*)lParam;
> +
> +        if (hdr->code == DTN_DROPDOWN)
> +        {
> +            g_hWndMonthCal = (HWND)SendMessageA(hdr->hwndFrom, DTM_GETMONTHCAL, 0, 0);
> +            trace("open SysMonthCal32=%p\n", g_hWndMonthCal );
> +        }
> +        else if (hdr->code == DTN_CLOSEUP)
> +        {
> +            trace("close SysMonthCal32=%p\n", g_hWndMonthCal);
> +        }
> +    }

That's not a notification test as you don't check anything. Please add a 
proper message sequence test, we have tons of examples for those.