public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
@ 2017-12-04 15:59 Ken Brown
  2017-12-05 12:58 ` Jon Turney
  0 siblings, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-12-04 15:59 UTC (permalink / raw)
  To: cygwin-apps

---
 site.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/site.cc b/site.cc
index 641a6bb..64e56a8 100644
--- a/site.cc
+++ b/site.cc
@@ -759,7 +759,9 @@ bool SitePage::OnMessageCmd (int id, HWND hwndctl, UINT code)
     {
     case IDC_EDIT_USER_URL:
       {
-	// FIXME: Make Enter here cause an ADD, not a NEXT.
+	// Set the default pushbutton to ADD if the user is entering text.
+	if (code == EN_CHANGE)
+	  SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) IDC_BUTTON_ADD_URL, 0);
 	break;
       }
     case IDC_URL_LIST:
-- 
2.15.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-04 15:59 [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT Ken Brown
@ 2017-12-05 12:58 ` Jon Turney
  2017-12-05 16:03   ` Ken Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Jon Turney @ 2017-12-05 12:58 UTC (permalink / raw)
  To: cygwin-apps

On 04/12/2017 15:58, Ken Brown wrote:
> ---
>   site.cc | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/site.cc b/site.cc
> index 641a6bb..64e56a8 100644
> --- a/site.cc
> +++ b/site.cc
> @@ -759,7 +759,9 @@ bool SitePage::OnMessageCmd (int id, HWND hwndctl, UINT code)
>       {
>       case IDC_EDIT_USER_URL:
>         {
> -	// FIXME: Make Enter here cause an ADD, not a NEXT.
> +	// Set the default pushbutton to ADD if the user is entering text.
> +	if (code == EN_CHANGE)
> +	  SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) IDC_BUTTON_ADD_URL, 0);
>   	break;
>         }
>       case IDC_URL_LIST:
> 

Very nice.  That fixme has been there since 2002 (and the bug probably 
longer...)

I thought perhaps we might need to reset the default control if the 
focus is moved to another control after IDC_EDIT_USER_URL so that enter 
works correctly then, but that doesn't seem to be the case.

Please apply.

The search textbox on the package chooser page needs the same fix.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-05 12:58 ` Jon Turney
@ 2017-12-05 16:03   ` Ken Brown
  2017-12-05 17:32     ` Ken Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-12-05 16:03 UTC (permalink / raw)
  To: cygwin-apps

On 12/5/2017 7:58 AM, Jon Turney wrote:
> Please apply.

Done.

> The search textbox on the package chooser page needs the same fix.

It's not immediately clear to me how to do this, since I don't know what 
the default pushbutton should be while the user is typing in the search box.

One possibility is to convert the label "Search" to the left of the box 
to a SEARCH pushbutton, whose effect is to call OnTimerMessage().  If we 
make this the default, then pressing Enter will cause the search filter 
to immediately take effect, which is probably what the user expects.

Or do you have a better idea?

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-05 16:03   ` Ken Brown
@ 2017-12-05 17:32     ` Ken Brown
  2017-12-07 18:35       ` Jon Turney
  0 siblings, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-12-05 17:32 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On 12/5/2017 11:03 AM, Ken Brown wrote:
> On 12/5/2017 7:58 AM, Jon Turney wrote:
>> Please apply.
> 
> Done.
> 
>> The search textbox on the package chooser page needs the same fix.
> 
> It's not immediately clear to me how to do this, since I don't know what 
> the default pushbutton should be while the user is typing in the search 
> box.
> 
> One possibility is to convert the label "Search" to the left of the box 
> to a SEARCH pushbutton, whose effect is to call OnTimerMessage().  If we 
> make this the default, then pressing Enter will cause the search filter 
> to immediately take effect, which is probably what the user expects.

Something like the attached?  This might not be quite right, because the 
previous default button is never restored.  I'm not sure how important 
that is.

Ken


[-- Attachment #2: 0001-Fix-response-to-Enter-in-the-chooser-textbox.patch --]
[-- Type: text/plain, Size: 3046 bytes --]

From e7f96c2db95c8bb4e9eb3a91730db7328b6e7d12 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Tue, 5 Dec 2017 12:21:12 -0500
Subject: [PATCH] Fix response to Enter in the chooser textbox

Make Enter cause the search filter to immediately take effect.
Previously Enter would cause NEXT.
---
 choose.cc  |  5 +++++
 res.rc     | 10 +++++-----
 resource.h |  1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/choose.cc b/choose.cc
index 1bc4c0b..0c6704f 100644
--- a/choose.cc
+++ b/choose.cc
@@ -386,6 +386,7 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
 
   if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT)
     {
+      SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) IDC_CHOOSE_DO_SEARCH, 0);
       SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL);
       return true;
     }
@@ -402,6 +403,10 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
       }
       break;
 
+    case IDC_CHOOSE_DO_SEARCH:
+      SendMessage (GetHWND (), WM_TIMER, (WPARAM) timer_id, 0);
+      break;
+
     case IDC_CHOOSE_KEEP:
       if (IsButtonChecked (id))
         keepClicked();
diff --git a/res.rc b/res.rc
index a4d7e70..8839196 100644
--- a/res.rc
+++ b/res.rc
@@ -323,11 +323,11 @@ END
 #define SETUP_VIEW_W		(20)
 #define SETUP_VIEWLIST_X		(SETUP_VIEW_X + SETUP_VIEW_W + 2)
 #define SETUP_VIEWLIST_W		(60)
-#define SETUP_SEARCH_X		(SETUP_VIEWLIST_X + SETUP_VIEWLIST_W + 2)
+#define SETUP_SEARCH_X		(SETUP_SEARCHTEXT_X + SETUP_SEARCHTEXT_W + 2)
 #define SETUP_SEARCH_W		(32)
-#define SETUP_SEARCHTEXT_X	(SETUP_SEARCH_X + SETUP_SEARCH_W + 2)
+#define SETUP_SEARCHTEXT_X	(SETUP_VIEWLIST_X + SETUP_VIEWLIST_W + 2)
 #define SETUP_SEARCHTEXT_W	(60)
-#define SETUP_CLEAR_X		(SETUP_SEARCHTEXT_X + SETUP_SEARCHTEXT_W + 2)
+#define SETUP_CLEAR_X		(SETUP_SEARCH_X + SETUP_SEARCH_W + 2)
 #define SETUP_CLEAR_W		(22)
 
 IDD_CHOOSE DIALOG DISCARDABLE  0, 0, SETUP_STANDARD_DIALOG_DIMS
@@ -340,10 +340,10 @@ BEGIN
                     SETUP_VIEW_W, 10
     COMBOBOX        IDC_CHOOSE_VIEW, SETUP_VIEWLIST_X, 30, SETUP_VIEWLIST_W, 84,
                     CBS_DROPDOWNLIST | WS_TABSTOP
-    RTEXT           "&Search", IDC_STATIC, SETUP_SEARCH_X, 33, SETUP_SEARCH_W,
-                    10, SS_CENTERIMAGE, WS_EX_RIGHT
     EDITTEXT        IDC_CHOOSE_SEARCH_EDIT, SETUP_SEARCHTEXT_X, 30,
                     SETUP_SEARCHTEXT_W, 14, ES_AUTOHSCROLL
+    PUSHBUTTON      "&Search", IDC_CHOOSE_DO_SEARCH, SETUP_SEARCH_X, 30,
+                    SETUP_SEARCH_W, 14
     PUSHBUTTON      "&Clear", IDC_CHOOSE_CLEAR_SEARCH, SETUP_CLEAR_X, 30,
                     SETUP_CLEAR_W, 14
     CONTROL         "&Keep", IDC_CHOOSE_KEEP, "Button", BS_AUTORADIOBUTTON
diff --git a/resource.h b/resource.h
index a2e867f..79b876d 100644
--- a/resource.h
+++ b/resource.h
@@ -177,3 +177,4 @@
 #define IDC_FILE_INUSE_HELP               592
 #define IDC_NET_DIRECT_LEGACY             593
 #define IDC_DOWNLOAD_EDIT                 594
+#define IDC_CHOOSE_DO_SEARCH              595
-- 
2.15.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-05 17:32     ` Ken Brown
@ 2017-12-07 18:35       ` Jon Turney
  2017-12-07 20:46         ` Ken Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Jon Turney @ 2017-12-07 18:35 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On 05/12/2017 17:32, Ken Brown wrote:
> On 12/5/2017 11:03 AM, Ken Brown wrote:
>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>> The search textbox on the package chooser page needs the same fix.
>>
>> It's not immediately clear to me how to do this, since I don't know 
>> what the default pushbutton should be while the user is typing in the 
>> search box.
>>
>> One possibility is to convert the label "Search" to the left of the 
>> box to a SEARCH pushbutton, whose effect is to call OnTimerMessage().  
>> If we make this the default, then pressing Enter will cause the search 
>> filter to immediately take effect, which is probably what the user 
>> expects.

It seems a bit weird to have a button which automatically pushes itself 
half a second after you finish typing.

Attached is my attempt, which (ab)uses an invisible button.

> Something like the attached?  This might not be quite right, because the 
> previous default button is never restored.  I'm not sure how important 
> that is.

I think it's something that should be done, if possible, so I added that.

I'm a bit puzzled that IDD_SITE apparently doesn't need anything to do 
that, but IDD_CHOOSE does...

[-- Attachment #2: 0001-Fix-response-to-enter-in-the-chooser-search-textbox.patch --]
[-- Type: text/plain, Size: 3210 bytes --]

From c403b87ac035c9d7678a60764e0f39cbaf85c435 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Tue, 5 Dec 2017 12:21:12 -0500
Subject: [PATCH setup] Fix response to enter in the chooser search textbox

Make 'enter' after we've started typing into the search textbox cause the
search filter to immediately take effect.

Note that we don't change the default control immediately on EN_SETFOCUS,
but wait for ENCHANGE, so pressing 'enter' before typing anything into the
search textbox still moves to the next page.

Also improve a bit of debug output from ChooserPage::OnMessageCmd()
---
 choose.cc  | 20 +++++++++++++++++---
 res.rc     |  2 ++
 resource.h |  1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/choose.cc b/choose.cc
index 1bc4c0b..4219a81 100644
--- a/choose.cc
+++ b/choose.cc
@@ -381,12 +381,19 @@ bool
 ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
 {
 #if DEBUG
-  Log (LOG_BABBLE) << "OnMesageCmd " << id << " " << hwndctl << " " << code << endLog;
+  Log (LOG_BABBLE) << "OnMessageCmd " << id << " " << hwndctl << " " << std::hex << code << endLog;
 #endif
 
-  if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT)
+  if (id == IDC_CHOOSE_SEARCH_EDIT)
     {
-      SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL);
+      if (code == EN_CHANGE)
+        {
+          SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) IDC_CHOOSE_DO_SEARCH, 0);
+          SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL);
+        }
+      else if (code == EN_KILLFOCUS)
+        SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) 0x3024 /* ID_WIZNEXT */, 0);
+
       return true;
     }
   else if (code == BN_CLICKED)
@@ -402,6 +409,13 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
       }
       break;
 
+    case IDC_CHOOSE_DO_SEARCH:
+      // invisible pushbutton which is the default pushbutton while typing into
+      // the search textbox, so that 'enter' causes the filter to be applied
+      // immediately, rather than activating the next page
+      SendMessage (GetHWND (), WM_TIMER, (WPARAM) timer_id, 0);
+      break;
+
     case IDC_CHOOSE_KEEP:
       if (IsButtonChecked (id))
         keepClicked();
diff --git a/res.rc b/res.rc
index a4d7e70..901cf76 100644
--- a/res.rc
+++ b/res.rc
@@ -342,6 +342,8 @@ BEGIN
                     CBS_DROPDOWNLIST | WS_TABSTOP
     RTEXT           "&Search", IDC_STATIC, SETUP_SEARCH_X, 33, SETUP_SEARCH_W,
                     10, SS_CENTERIMAGE, WS_EX_RIGHT
+    CONTROL         "Search ", IDC_CHOOSE_DO_SEARCH, "Button", BS_PUSHBUTTON | NOT
+                    WS_VISIBLE, SETUP_SEARCH_X, 33, SETUP_SEARCH_W, 14
     EDITTEXT        IDC_CHOOSE_SEARCH_EDIT, SETUP_SEARCHTEXT_X, 30,
                     SETUP_SEARCHTEXT_W, 14, ES_AUTOHSCROLL
     PUSHBUTTON      "&Clear", IDC_CHOOSE_CLEAR_SEARCH, SETUP_CLEAR_X, 30,
diff --git a/resource.h b/resource.h
index a2e867f..79b876d 100644
--- a/resource.h
+++ b/resource.h
@@ -177,3 +177,4 @@
 #define IDC_FILE_INUSE_HELP               592
 #define IDC_NET_DIRECT_LEGACY             593
 #define IDC_DOWNLOAD_EDIT                 594
+#define IDC_CHOOSE_DO_SEARCH              595
-- 
2.15.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-07 18:35       ` Jon Turney
@ 2017-12-07 20:46         ` Ken Brown
  2017-12-08  4:55           ` Brian Inglis
  2017-12-08 15:48           ` Jon Turney
  0 siblings, 2 replies; 11+ messages in thread
From: Ken Brown @ 2017-12-07 20:46 UTC (permalink / raw)
  To: cygwin-apps

On 12/7/2017 1:35 PM, Jon Turney wrote:
> On 05/12/2017 17:32, Ken Brown wrote:
>> On 12/5/2017 11:03 AM, Ken Brown wrote:
>>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>>> The search textbox on the package chooser page needs the same fix.
>>>
>>> It's not immediately clear to me how to do this, since I don't know 
>>> what the default pushbutton should be while the user is typing in the 
>>> search box.
>>>
>>> One possibility is to convert the label "Search" to the left of the 
>>> box to a SEARCH pushbutton, whose effect is to call OnTimerMessage(). 
>>> If we make this the default, then pressing Enter will cause the 
>>> search filter to immediately take effect, which is probably what the 
>>> user expects.
> 
> It seems a bit weird to have a button which automatically pushes itself 
> half a second after you finish typing.
> 
> Attached is my attempt, which (ab)uses an invisible button.

I agree, this is better than my version.

>> Something like the attached?  This might not be quite right, because 
>> the previous default button is never restored.  I'm not sure how 
>> important that is.
> 
> I think it's something that should be done, if possible, so I added that.

In my testing, 'Next' does indeed become the default button after I 
click outside of the textbox, but there's no visual indication of this.

The MSDN page for DM_SETDEFID 
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms645413(v=vs.85).aspx) 
mentions a different situation where button highlighting doesn't 
accurately reflect the default button.  In that case it suggests sending 
a BM_SETSTYLE message to change the border style.

I looked at the documentation for BM_SETSTYLE, but it wasn't obvious to 
me what do do.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-07 20:46         ` Ken Brown
@ 2017-12-08  4:55           ` Brian Inglis
  2017-12-08 14:47             ` Ken Brown
  2017-12-08 15:48           ` Jon Turney
  1 sibling, 1 reply; 11+ messages in thread
From: Brian Inglis @ 2017-12-08  4:55 UTC (permalink / raw)
  To: cygwin-apps

On 2017-12-07 13:46, Ken Brown wrote:
> On 12/7/2017 1:35 PM, Jon Turney wrote:
>> On 05/12/2017 17:32, Ken Brown wrote:
>>> On 12/5/2017 11:03 AM, Ken Brown wrote:
>>>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>>>> The search textbox on the package chooser page needs the same fix.
>>>> It's not immediately clear to me how to do this, since I don't know what the
>>>> default pushbutton should be while the user is typing in the search box.
>>>> One possibility is to convert the label "Search" to the left of the box to a
>>>> SEARCH pushbutton, whose effect is to call OnTimerMessage(). If we make this
>>>> the default, then pressing Enter will cause the search filter to immediately
>>>> take effect, which is probably what the user expects.
>> It seems a bit weird to have a button which automatically pushes itself half a
>> second after you finish typing.
>> Attached is my attempt, which (ab)uses an invisible button.
> I agree, this is better than my version.
>>> Something like the attached?  This might not be quite right, because the
>>> previous default button is never restored.  I'm not sure how important that is.
>> I think it's something that should be done, if possible, so I added that.
> In my testing, 'Next' does indeed become the default button after I click
> outside of the textbox, but there's no visual indication of this.
> The MSDN page for DM_SETDEFID
> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms645413(v=vs.85).aspx) mentions
> a different situation where button highlighting doesn't accurately reflect the
> default button.  In that case it suggests sending a BM_SETSTYLE message to
> change the border style.
> I looked at the documentation for BM_SETSTYLE, but it wasn't obvious to me what
> do do.

Seems like you want to change the button state not style - checking around you
might want to use Button_SetState macro or BM_SETSTATE message TRUE to
highlight, FALSE to reset:

https://msdn.microsoft.com/en-us/library/windows/desktop/bb849168(v=vs.85).aspx

with Button_SetStyle macro or BM_SETSTYLE message style BS_DEFPUSHBUTTON |
BS_CENTER | BS_VCENTER | BS_TEXT | ...  as appropriate, so that ENTER works like
a click:

https://msdn.microsoft.com/en-us/library/windows/desktop/bb775951(v=vs.85).aspx

When content is first added to the Search text box and while content exists, you
may want to BM_SETSTATE FALSE and clear BS_DEFPUSHBUTTON on [Next], then after
the next action, BM_SETSTATE TRUE and set BS_DEFPUSHBUTTON on [Next].

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-08  4:55           ` Brian Inglis
@ 2017-12-08 14:47             ` Ken Brown
  2017-12-08 15:55               ` Jon Turney
  0 siblings, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-12-08 14:47 UTC (permalink / raw)
  To: cygwin-apps

On 12/7/2017 11:55 PM, Brian Inglis wrote:
> On 2017-12-07 13:46, Ken Brown wrote:
>> On 12/7/2017 1:35 PM, Jon Turney wrote:
>>> On 05/12/2017 17:32, Ken Brown wrote:
>>>> On 12/5/2017 11:03 AM, Ken Brown wrote:
>>>>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>>>>> The search textbox on the package chooser page needs the same fix.
>>>>> It's not immediately clear to me how to do this, since I don't know what the
>>>>> default pushbutton should be while the user is typing in the search box.
>>>>> One possibility is to convert the label "Search" to the left of the box to a
>>>>> SEARCH pushbutton, whose effect is to call OnTimerMessage(). If we make this
>>>>> the default, then pressing Enter will cause the search filter to immediately
>>>>> take effect, which is probably what the user expects.
>>> It seems a bit weird to have a button which automatically pushes itself half a
>>> second after you finish typing.
>>> Attached is my attempt, which (ab)uses an invisible button.
>> I agree, this is better than my version.
>>>> Something like the attached?  This might not be quite right, because the
>>>> previous default button is never restored.  I'm not sure how important that is.
>>> I think it's something that should be done, if possible, so I added that.
>> In my testing, 'Next' does indeed become the default button after I click
>> outside of the textbox, but there's no visual indication of this.
>> The MSDN page for DM_SETDEFID
>> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms645413(v=vs.85).aspx) mentions
>> a different situation where button highlighting doesn't accurately reflect the
>> default button.  In that case it suggests sending a BM_SETSTYLE message to
>> change the border style.
>> I looked at the documentation for BM_SETSTYLE, but it wasn't obvious to me what
>> do do.
> 
> Seems like you want to change the button state not style - checking around you
> might want to use Button_SetState macro or BM_SETSTATE message TRUE to
> highlight, FALSE to reset:
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/bb849168(v=vs.85).aspx
> 
> with Button_SetStyle macro or BM_SETSTYLE message style BS_DEFPUSHBUTTON |
> BS_CENTER | BS_VCENTER | BS_TEXT | ...  as appropriate, so that ENTER works like
> a click:
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/bb775951(v=vs.85).aspx

Thanks for the suggestions, but I tried various BM_SETSTATE and 
BM_SETSTYLE messages, and I couldn't find anything that worked: 'Next' 
remained unhighlighted.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-07 20:46         ` Ken Brown
  2017-12-08  4:55           ` Brian Inglis
@ 2017-12-08 15:48           ` Jon Turney
  2017-12-08 16:14             ` Ken Brown
  1 sibling, 1 reply; 11+ messages in thread
From: Jon Turney @ 2017-12-08 15:48 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1974 bytes --]

On 07/12/2017 20:46, Ken Brown wrote:
> On 12/7/2017 1:35 PM, Jon Turney wrote:
>> On 05/12/2017 17:32, Ken Brown wrote:
>>> On 12/5/2017 11:03 AM, Ken Brown wrote:
>>>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>>>> The search textbox on the package chooser page needs the same fix.
>>>>
>>>> It's not immediately clear to me how to do this, since I don't know 
>>>> what the default pushbutton should be while the user is typing in 
>>>> the search box.
>>>>
>>>> One possibility is to convert the label "Search" to the left of the 
>>>> box to a SEARCH pushbutton, whose effect is to call 
>>>> OnTimerMessage(). If we make this the default, then pressing Enter 
>>>> will cause the search filter to immediately take effect, which is 
>>>> probably what the user expects.
>>
>> It seems a bit weird to have a button which automatically pushes 
>> itself half a second after you finish typing.
>>
>> Attached is my attempt, which (ab)uses an invisible button.
> 
> I agree, this is better than my version.
> 
>>> Something like the attached?  This might not be quite right, because 
>>> the previous default button is never restored.  I'm not sure how 
>>> important that is.
>>
>> I think it's something that should be done, if possible, so I added that.
> 
> In my testing, 'Next' does indeed become the default button after I 
> click outside of the textbox, but there's no visual indication of this.

This is interesting: if you use TAB to move the focus out of the 
textbox, then first "Clear" gets highlight (because it's a pushbutton 
and enter pushes it), TAB again and "Current is selected (but "Next" 
gets the highlight, because that's what enter pushes)

If you click to move the focus, it only seems to update the highlight 
the second time you do that.

Which I guess suggests we should be ensuring the highlight is drawn on 
EN_KILLFOCUS?

But once I do that, it seems I need to explicitly remove as well, which 
gives the attached, incremental patch.

[-- Attachment #2: 0001-Explicitly-remove-defpushbutton-style-from-Next-butt.patch --]
[-- Type: text/plain, Size: 1440 bytes --]

From 4308c448d33e5de993586a4573f6810efdd26bbf Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Fri, 8 Dec 2017 15:44:03 +0000
Subject: [PATCH setup] Explicitly remove defpushbutton style from "Next"
 button when not default

---
 choose.cc | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/choose.cc b/choose.cc
index 4219a81..d7265cb 100644
--- a/choose.cc
+++ b/choose.cc
@@ -27,6 +27,7 @@
    default. */
 
 #include "win32.h"
+#include "windowsx.h"
 #include <commctrl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -386,14 +387,19 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
 
   if (id == IDC_CHOOSE_SEARCH_EDIT)
     {
+      HWND nextButton = ::GetDlgItem(::GetParent(GetHWND()), 0x3024 /* ID_WIZNEXT */);
+
       if (code == EN_CHANGE)
         {
           SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) IDC_CHOOSE_DO_SEARCH, 0);
+          Button_SetStyle(nextButton, BS_PUSHBUTTON, TRUE);
           SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL);
         }
       else if (code == EN_KILLFOCUS)
-        SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) 0x3024 /* ID_WIZNEXT */, 0);
-
+        {
+          SendMessage (GetHWND (), DM_SETDEFID, (WPARAM) 0x3024 /* ID_WIZNEXT */, 0);
+          Button_SetStyle(nextButton, BS_DEFPUSHBUTTON, TRUE);
+        }
       return true;
     }
   else if (code == BN_CLICKED)
-- 
2.15.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-08 14:47             ` Ken Brown
@ 2017-12-08 15:55               ` Jon Turney
  0 siblings, 0 replies; 11+ messages in thread
From: Jon Turney @ 2017-12-08 15:55 UTC (permalink / raw)
  To: cygwin-apps

On 08/12/2017 14:47, Ken Brown wrote:
> On 12/7/2017 11:55 PM, Brian Inglis wrote:
>> On 2017-12-07 13:46, Ken Brown wrote:
>>> In my testing, 'Next' does indeed become the default button after I 
>>> click
>>> outside of the textbox, but there's no visual indication of this.
>>> The MSDN page for DM_SETDEFID
>>> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms645413(v=vs.85).aspx) 
>>> mentions
>>> a different situation where button highlighting doesn't accurately 
>>> reflect the
>>> default button.  In that case it suggests sending a BM_SETSTYLE 
>>> message to
>>> change the border style.
>>> I looked at the documentation for BM_SETSTYLE, but it wasn't obvious 
>>> to me what
>>> do do.
>>
>> Seems like you want to change the button state not style - checking 
>> around you
>> might want to use Button_SetState macro or BM_SETSTATE message TRUE to
>> highlight, FALSE to reset:
>>
>> https://msdn.microsoft.com/en-us/library/windows/desktop/bb849168(v=vs.85).aspx 

No, 'state' is the highlight when the button is pressed, which is 
different to the outline 'style' when the button is the default (ofc, 
MSDN documentation is as clear as mud, as usual)

> Thanks for the suggestions, but I tried various BM_SETSTATE and 
> BM_SETSTYLE messages, and I couldn't find anything that worked: 'Next' 
> remained unhighlighted.

The trick here, as you might notice from the patch in my reply, is that 
there's a window hierarchy involved: the IDD_CHOOSE window is embedded 
in the propsheet window, which owns the 'next' control.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT
  2017-12-08 15:48           ` Jon Turney
@ 2017-12-08 16:14             ` Ken Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Ken Brown @ 2017-12-08 16:14 UTC (permalink / raw)
  To: cygwin-apps

On 12/8/2017 10:48 AM, Jon Turney wrote:
> On 07/12/2017 20:46, Ken Brown wrote:
>> On 12/7/2017 1:35 PM, Jon Turney wrote:
>>> On 05/12/2017 17:32, Ken Brown wrote:
>>>> On 12/5/2017 11:03 AM, Ken Brown wrote:
>>>>> On 12/5/2017 7:58 AM, Jon Turney wrote:
>>>>>> The search textbox on the package chooser page needs the same fix.
>>>>>
>>>>> It's not immediately clear to me how to do this, since I don't know 
>>>>> what the default pushbutton should be while the user is typing in 
>>>>> the search box.
>>>>>
>>>>> One possibility is to convert the label "Search" to the left of the 
>>>>> box to a SEARCH pushbutton, whose effect is to call 
>>>>> OnTimerMessage(). If we make this the default, then pressing Enter 
>>>>> will cause the search filter to immediately take effect, which is 
>>>>> probably what the user expects.
>>>
>>> It seems a bit weird to have a button which automatically pushes 
>>> itself half a second after you finish typing.
>>>
>>> Attached is my attempt, which (ab)uses an invisible button.
>>
>> I agree, this is better than my version.
>>
>>>> Something like the attached?  This might not be quite right, because 
>>>> the previous default button is never restored.  I'm not sure how 
>>>> important that is.
>>>
>>> I think it's something that should be done, if possible, so I added 
>>> that.
>>
>> In my testing, 'Next' does indeed become the default button after I 
>> click outside of the textbox, but there's no visual indication of this.
> 
> This is interesting: if you use TAB to move the focus out of the 
> textbox, then first "Clear" gets highlight (because it's a pushbutton 
> and enter pushes it), TAB again and "Current is selected (but "Next" 
> gets the highlight, because that's what enter pushes)
> 
> If you click to move the focus, it only seems to update the highlight 
> the second time you do that.
> 
> Which I guess suggests we should be ensuring the highlight is drawn on 
> EN_KILLFOCUS?
> 
> But once I do that, it seems I need to explicitly remove as well, which 
> gives the attached, incremental patch.

That seems to fix it.  Thanks.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-12-08 16:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04 15:59 [PATCH setup] Make Enter in the user URL box cause ADD instead of NEXT Ken Brown
2017-12-05 12:58 ` Jon Turney
2017-12-05 16:03   ` Ken Brown
2017-12-05 17:32     ` Ken Brown
2017-12-07 18:35       ` Jon Turney
2017-12-07 20:46         ` Ken Brown
2017-12-08  4:55           ` Brian Inglis
2017-12-08 14:47             ` Ken Brown
2017-12-08 15:55               ` Jon Turney
2017-12-08 15:48           ` Jon Turney
2017-12-08 16:14             ` Ken Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).