* [PATCH setup 0/4] Use a pop-up menu to select chooser view filter
@ 2016-08-24 14:16 Jon Turney
2016-08-24 14:16 ` [PATCH setup 4/4] Use a pop-up menu to directly " Jon Turney
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Jon Turney @ 2016-08-24 14:16 UTC (permalink / raw)
To: cygwin-apps; +Cc: Jon Turney
Clicking on a button to cycle around alternatives seems bad UI, so instead allow
a specific filter view to be directly selected with a pop-up menu.
I'm not sure if this is a great improvement or not, but since I wrote it, here
it is...
Jon Turney (4):
Build C++ code with -std=gnu++11
Change PickView::view into an enum
Rename PickView::Package to PickView::PackagePending
Use a pop-up menu to directly select chooser view filter
Makefile.am | 9 +++--
PickView.cc | 52 ++++++++--------------------
PickView.h | 43 ++++++------------------
choose.cc | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
choose.h | 1 +
ini.cc | 2 +-
res.rc | 22 ++++++++++--
resource.h | 10 ++++++
8 files changed, 164 insertions(+), 85 deletions(-)
--
2.8.3
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH setup 4/4] Use a pop-up menu to directly select chooser view filter 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney @ 2016-08-24 14:16 ` Jon Turney 2016-08-24 14:16 ` [PATCH setup 2/4] Change PickView::view into an enum Jon Turney ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-24 14:16 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Rather that cycling through views, popup a menu to choose the view when the view button is clicked Future work: Hopefully this does reduce the average number of clicks needed, but popping-up on mouse button down, rather than mouse button click is perhaps what we really want here, but seems to be rather hard to achieve in a propsheet. --- PickView.cc | 16 ++++----- PickView.h | 2 +- choose.cc | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- choose.h | 1 + res.rc | 22 +++++++++++-- resource.h | 10 ++++++ 6 files changed, 141 insertions(+), 18 deletions(-) diff --git a/PickView.cc b/PickView.cc index 25d43a2..3702b92 100644 --- a/PickView.cc +++ b/PickView.cc @@ -141,16 +141,6 @@ PickView::note_width (PickView::Header *hdrs, HDC dc, } void -PickView::cycleViewMode () -{ - PickView::views _value = (PickView::views)((int)view_mode + 1); - if (_value > PickView::views::Category) - _value = PickView::views::PackageFull; - - setViewMode (_value); -} - -void PickView::setViewMode (views mode) { view_mode = mode; @@ -227,6 +217,12 @@ PickView::setViewMode (views mode) InvalidateRect (GetHWND(), &r, TRUE); } +PickView::views +PickView::getViewMode () +{ + return view_mode; +} + const char * PickView::mode_caption () { diff --git a/PickView.h b/PickView.h index c07249b..332383a 100644 --- a/PickView.h +++ b/PickView.h @@ -44,8 +44,8 @@ public: class Header; int num_columns; void defaultTrust (trusts trust); - void cycleViewMode (); void setViewMode (views mode); + views getViewMode (); void DrawIcon (HDC hdc, int x, int y, HANDLE hIcon); void paint (HWND hwnd); LRESULT CALLBACK list_click (HWND hwnd, BOOL dblclk, int x, int y, UINT hitCode); diff --git a/choose.cc b/choose.cc index 3c7f4f8..180c788 100644 --- a/choose.cc +++ b/choose.cc @@ -385,6 +385,10 @@ ChooserPage::changeTrust(trusts aTrust) bool ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) { +#if DEBUG + Log (LOG_BABBLE) << "OnMesageCmd " << id << " " << hwndctl << " " << code << endLog; +#endif + if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT) { SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL); @@ -423,11 +427,7 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) break; case IDC_CHOOSE_VIEW: - chooser->cycleViewMode (); - if (!SetDlgItemText - (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) - Log (LOG_BABBLE) << "Failed to set View button caption " << - GetLastError () << endLog; + selectView(); break; case IDC_CHOOSE_HIDE: @@ -442,6 +442,104 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) return true; } +static void +setMenuItemState (HMENU hMenu, UINT item, UINT state) +{ + MENUITEMINFO mii; + memset(&mii, 0, sizeof(MENUITEMINFO)); + mii.cbSize = sizeof(MENUITEMINFO); + mii.fMask = MIIM_STATE; + mii.fState = state; + SetMenuItemInfo (hMenu, item, FALSE, &mii); +} + +void +ChooserPage::selectView (void) +{ + HMENU hMenu = LoadMenu (GetModuleHandle (NULL), MAKEINTRESOURCE (IDM_CHOOSE_VIEW)); + hMenu = GetSubMenu (hMenu, 0); + + // mark the current view mode as selected + int item = 0; + PickView::views view_mode = chooser->getViewMode (); + + switch (view_mode) + { + case PickView::views::PackageFull: + item = IDM_VIEW_FULL; + break; + case PickView::views::PackagePending: + item = IDM_VIEW_PENDING; + break; + case PickView::views::PackageKeeps: + item = IDM_VIEW_UPTODATE; + break; + case PickView::views::PackageSkips: + item = IDM_VIEW_NOT_INSTALLED; + break; + case PickView::views::PackageUserPicked: + item = IDM_VIEW_PICKED; + break; + case PickView::views::Category: + item = IDM_VIEW_CATEGORY; + break; + case PickView::views::Unknown: + item = 0; + } + + if (item) + setMenuItemState (hMenu, item, MFS_CHECKED); + + // place the menu over the 'view' button + HWND hButton = ::GetDlgItem (GetHWND (), IDC_CHOOSE_VIEW); + RECT rect; + ::GetWindowRect (hButton, &rect); + + item = TrackPopupMenu (hMenu, + TPM_LEFTALIGN | TPM_TOPALIGN | TPM_RETURNCMD | TPM_LEFTBUTTON | TPM_NOANIMATION, + rect.left, rect.top, + 0, GetHWND (), NULL); + + DestroyMenu (hMenu); + +#if DEBUG + Log (LOG_BABBLE) << "TrackPopupMenu returned " << item << endLog; +#endif + + // menu cancelled without making a selection + if (item == 0) + return; + + // switch to the selected view + switch (item) + { + case IDM_VIEW_FULL: + view_mode = PickView::views::PackageFull; + break; + case IDM_VIEW_PENDING: + view_mode = PickView::views::PackagePending; + break; + case IDM_VIEW_UPTODATE: + view_mode = PickView::views::PackageKeeps; + break; + case IDM_VIEW_NOT_INSTALLED: + view_mode = PickView::views::PackageSkips; + break; + case IDM_VIEW_PICKED: + view_mode = PickView::views::PackageUserPicked; + break; + case IDM_VIEW_CATEGORY: + view_mode = PickView::views::Category; + break; + } + + chooser->setViewMode (view_mode); + if (!SetDlgItemText + (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) + Log (LOG_BABBLE) << "Failed to set View button caption " << + GetLastError () << endLog; +} + INT_PTR CALLBACK ChooserPage::OnMouseWheel (UINT message, WPARAM wParam, LPARAM lParam) { diff --git a/choose.h b/choose.h index 46f0f35..8832f5e 100644 --- a/choose.h +++ b/choose.h @@ -59,6 +59,7 @@ private: void logResults(); void setPrompt(char const *aPrompt); void PlaceDialog (bool); + void selectView (void); PickView *chooser; static HWND ins_dialog; diff --git a/res.rc b/res.rc index 2fae133..10ec375 100644 --- a/res.rc +++ b/res.rc @@ -536,8 +536,8 @@ BEGIN "considered the most stable. (RECOMMENDED)" IDS_TRUSTEXP_TOOLTIP "Globally select the most recent version, even if " "that version is considered Experimental or for test use by the maintainer." - IDS_VIEWBUTTON_TOOLTIP "Cycles the package view. This determines " - "which packages are shown in the chooser below.\r\n" + IDS_VIEWBUTTON_TOOLTIP "Select the package view. This determines " + "which packages are shown below.\r\n" "\r\n" "Category: Group by package category. Click on '+' to expand.\r\n" "\r\n" @@ -574,3 +574,21 @@ BEGIN IDS_ELEVATED "Hand installation over to elevated child process." IDS_INSTALLEDB_VERSION "Unknown INSTALLED.DB version" END + +///////////////////////////////////////////////////////////////////////////// +// +// Menu +// + +IDM_CHOOSE_VIEW MENUEX +BEGIN + POPUP "ViewMenu" + BEGIN + MENUITEM "&Full", IDM_VIEW_FULL, MFT_STRING | MFT_RADIOCHECK + MENUITEM "&Pending", IDM_VIEW_PENDING, MFT_STRING | MFT_RADIOCHECK + MENUITEM "&Up To Date", IDM_VIEW_UPTODATE, MFT_STRING | MFT_RADIOCHECK + MENUITEM "&Not Installed", IDM_VIEW_NOT_INSTALLED, MFT_STRING | MFT_RADIOCHECK + MENUITEM "P&icked", IDM_VIEW_PICKED, MFT_STRING | MFT_RADIOCHECK + MENUITEM "&Category", IDM_VIEW_CATEGORY, MFT_STRING | MFT_RADIOCHECK + END +END diff --git a/resource.h b/resource.h index 68e8023..e2312ac 100644 --- a/resource.h +++ b/resource.h @@ -175,3 +175,13 @@ #define IDC_FILE_INUSE_EDIT 590 #define IDC_FILE_INUSE_MSG 591 #define IDC_FILE_INUSE_HELP 592 + +// Menus + +#define IDM_CHOOSE_VIEW 900 +#define IDM_VIEW_FULL 901 +#define IDM_VIEW_PENDING 902 +#define IDM_VIEW_UPTODATE 903 +#define IDM_VIEW_NOT_INSTALLED 904 +#define IDM_VIEW_PICKED 905 +#define IDM_VIEW_CATEGORY 906 -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 2/4] Change PickView::view into an enum 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney 2016-08-24 14:16 ` [PATCH setup 4/4] Use a pop-up menu to directly " Jon Turney @ 2016-08-24 14:16 ` Jon Turney 2016-08-24 14:16 ` [PATCH setup 3/4] Rename PickView::Package to PickView::PackagePending Jon Turney ` (2 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-24 14:16 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Change PickView::view from a class, into an enum Use a C++11 scoped enum so we don't have to change all the uses of it's values. --- PickView.cc | 44 ++++++++++++-------------------------------- PickView.h | 41 +++++++++-------------------------------- 2 files changed, 21 insertions(+), 64 deletions(-) diff --git a/PickView.cc b/PickView.cc index c784a2a..588cfec 100644 --- a/PickView.cc +++ b/PickView.cc @@ -52,15 +52,6 @@ static PickView::Header cat_headers[] = { {0, 0, 0, false} }; -// PickView:: views -const PickView::views PickView::views::Unknown (0); -const PickView::views PickView::views::PackageFull (1); -const PickView::views PickView::views::Package (2); -const PickView::views PickView::views::PackageKeeps (3); -const PickView::views PickView::views::PackageSkips (4); -const PickView::views PickView::views::PackageUserPicked (5); -const PickView::views PickView::views::Category (6); - ATOM PickView::WindowClassAtom = 0; // DoInsertItem - inserts an item into a header control. @@ -152,7 +143,11 @@ PickView::note_width (PickView::Header *hdrs, HDC dc, void PickView::cycleViewMode () { - setViewMode (++view_mode); + PickView::views _value = (PickView::views)((int)view_mode + 1); + if (_value > PickView::views::Category) + _value = PickView::views::PackageFull; + + setViewMode (_value); } void @@ -235,25 +230,19 @@ PickView::setViewMode (views mode) const char * PickView::mode_caption () { - return view_mode.caption (); -} - -const char * -PickView::views::caption () -{ - switch (_value) + switch (view_mode) { - case 1: + case views::PackageFull: return "Full"; - case 2: + case views::Package: return "Pending"; - case 3: + case views::PackageKeeps: return "Up To Date"; - case 4: + case views::PackageSkips: return "Not Installed"; - case 5: + case views::PackageUserPicked: return "Picked"; - case 6: + case views::Category: return "Category"; default: return ""; @@ -348,15 +337,6 @@ PickView::insert_category (Category *cat, bool collapsed) delete &catline; } -PickView::views& -PickView::views::operator++ () -{ - ++_value; - if (_value > Category._value) - _value = 1; - return *this; -} - int PickView::click (int row, int x) { diff --git a/PickView.h b/PickView.h index fd20dc9..054ab4a 100644 --- a/PickView.h +++ b/PickView.h @@ -40,7 +40,7 @@ class PickView : public Window public: virtual bool Create (Window * Parent = NULL, DWORD Style = WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CLIPCHILDREN, RECT * r = NULL); virtual bool registerWindowClass (); - class views; + enum class views; class Header; int num_columns; void defaultTrust (trusts trust); @@ -96,38 +96,15 @@ public: return listheader; } - class views + enum class views { - public: - static const views Unknown; - static const views PackageFull; - static const views Package; - static const views PackageKeeps; - static const views PackageSkips; - static const views PackageUserPicked; - static const views Category; - views ():_value (0) - { - }; - views (int aInt) - { - _value = aInt; - if (_value < 0 || _value > 6) - _value = 0; - } - views & operator++ (); - bool operator == (views const &rhs) - { - return _value == rhs._value; - } - bool operator != (views const &rhs) - { - return _value != rhs._value; - } - const char *caption (); - - private: - int _value; + Unknown, + PackageFull, + Package, + PackageKeeps, + PackageSkips, + PackageUserPicked, + Category, }; class Header -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 3/4] Rename PickView::Package to PickView::PackagePending 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney 2016-08-24 14:16 ` [PATCH setup 4/4] Use a pop-up menu to directly " Jon Turney 2016-08-24 14:16 ` [PATCH setup 2/4] Change PickView::view into an enum Jon Turney @ 2016-08-24 14:16 ` Jon Turney 2016-08-24 14:16 ` [PATCH setup 1/4] Build C++ code with -std=gnu++11 Jon Turney 2016-08-24 16:50 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 4 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-24 14:16 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Rename PickView::Package to PickView::PackagePending, as that's what that view is --- PickView.cc | 8 ++++---- PickView.h | 2 +- choose.cc | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/PickView.cc b/PickView.cc index 588cfec..25d43a2 100644 --- a/PickView.cc +++ b/PickView.cc @@ -83,7 +83,7 @@ PickView::set_header_column_order (views vm) { if (vm == views::Unknown) return -1; - else if (vm == views::PackageFull || vm == views::Package + else if (vm == views::PackageFull || vm == views::PackagePending || vm == views::PackageKeeps || vm == views::PackageSkips || vm == views::PackageUserPicked) { @@ -181,7 +181,7 @@ PickView::setViewMode (views mode) (view_mode == PickView::views::PackageFull) // "Pending" : packages that are being added/removed/upgraded - || (view_mode == PickView::views::Package && + || (view_mode == PickView::views::PackagePending && ((!pkg.desired && pkg.installed) || // uninstall (pkg.desired && (pkg.desired.picked () || // install bin @@ -234,7 +234,7 @@ PickView::mode_caption () { case views::PackageFull: return "Full"; - case views::Package: + case views::PackagePending: return "Pending"; case views::PackageKeeps: return "Up To Date"; @@ -815,7 +815,7 @@ PickView::WindowProc (UINT message, WPARAM wParam, LPARAM lParam) lastWindowRect.width ()) != 0) { cat_headers[set_header_column_order (views::Category)].width += dx; - pkg_headers[set_header_column_order (views::Package)].width += dx; + pkg_headers[set_header_column_order (views::PackagePending)].width += dx; set_header_column_order (view_mode); set_headers (); ::MoveWindow (listheader, -scroll_ulc_x, 0, diff --git a/PickView.h b/PickView.h index 054ab4a..c07249b 100644 --- a/PickView.h +++ b/PickView.h @@ -100,7 +100,7 @@ public: { Unknown, PackageFull, - Package, + PackagePending, PackageKeeps, PackageSkips, PackageUserPicked, diff --git a/choose.cc b/choose.cc index 1c97f59..3c7f4f8 100644 --- a/choose.cc +++ b/choose.cc @@ -152,7 +152,7 @@ ChooserPage::createListview () chooser->init(PickView::views::Category); chooser->Show(SW_SHOW); chooser->setViewMode (UpgradeAlsoOption || hasManualSelections ? - PickView::views::Package : PickView::views::Category); + PickView::views::PackagePending : PickView::views::Category); if (!SetDlgItemText (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) Log (LOG_BABBLE) << "Failed to set View button caption %ld" << GetLastError () << endLog; -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 1/4] Build C++ code with -std=gnu++11 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney ` (2 preceding siblings ...) 2016-08-24 14:16 ` [PATCH setup 3/4] Rename PickView::Package to PickView::PackagePending Jon Turney @ 2016-08-24 14:16 ` Jon Turney 2016-08-24 16:50 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 4 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-24 14:16 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Use BASECXXFLAGS rather then AM_CXXFLAGS to make AM_CFLAGS Rationalize BASECXXFLAGS, adding -Werror and removing -Wno-uninitialized (since the bug preventing it being used is long fixed) Fix a bug detected by -Wuninitialized Build C++ code with -std=gnu++11 -Wno-deprecated-declarations --- Makefile.am | 9 ++++----- ini.cc | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7fa61e9..044a1ce 100644 --- a/Makefile.am +++ b/Makefile.am @@ -21,13 +21,12 @@ SUBDIRS := @subdirs@ tests # We would like to use -Winline for C++ as well, but some STL code triggers # this warning. (Bug verified present in gcc-3.3) -# -Wno-uninitialized added to deal with g++ 3.4.4's spurious STL warnings -# (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22207) -BASECXXFLAGS = -Wall -Wno-uninitialized -Wpointer-arith -Wcomments \ +BASECXXFLAGS = -Werror -Wall -Wpointer-arith -Wcomments \ -Wcast-align -Wwrite-strings -fno-builtin-sscanf \ -Wno-attributes -AM_CXXFLAGS = -Werror $(BASECXXFLAGS) ${$(*F)_CXXFLAGS} -AM_CFLAGS = $(AM_CXXFLAGS) -Wmissing-declarations -Winline \ +AM_CXXFLAGS = $(BASECXXFLAGS) -std=gnu++11 -Wno-deprecated-declarations \ + ${$(*F)_CXXFLAGS} +AM_CFLAGS = $(BASECXXFLAGS) -Wmissing-declarations -Winline \ -Wstrict-prototypes -Wmissing-prototypes AM_YFLAGS = -d AM_LFLAGS = -8 diff --git a/ini.cc b/ini.cc index f925bf5..82990a2 100644 --- a/ini.cc +++ b/ini.cc @@ -270,7 +270,7 @@ do_remote_ini (HWND owner) size_t ini_count = 0; GuiParseFeedback myFeedback; IniDBBuilderPackage aBuilder (myFeedback); - io_stream *ini_file, *ini_sig_file; + io_stream *ini_file = NULL, *ini_sig_file; /* FIXME: Get rid of this io_stream pointer travesty. The need to explicitly delete these things is ridiculous. */ -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney ` (3 preceding siblings ...) 2016-08-24 14:16 ` [PATCH setup 1/4] Build C++ code with -std=gnu++11 Jon Turney @ 2016-08-24 16:50 ` Corinna Vinschen 2016-08-25 19:11 ` Jon Turney 4 siblings, 1 reply; 17+ messages in thread From: Corinna Vinschen @ 2016-08-24 16:50 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 773 bytes --] On Aug 24 15:15, Jon Turney wrote: > Clicking on a button to cycle around alternatives seems bad UI, so instead allow > a specific filter view to be directly selected with a pop-up menu. > > I'm not sure if this is a great improvement or not, but since I wrote it, here > it is... Nice idea and works fine in my limited testing. One question: Usually the popup menu itself reflects the current choice. So, wouldn't it make more sense to have the text "View" just as plain text to the left, and the actual popup menu next right to it, showing the current choice? Yes? No? Shut up? Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-24 16:50 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen @ 2016-08-25 19:11 ` Jon Turney 2016-08-25 20:34 ` Yaakov Selkowitz 0 siblings, 1 reply; 17+ messages in thread From: Jon Turney @ 2016-08-25 19:11 UTC (permalink / raw) To: cygwin-apps On 24/08/2016 17:50, Corinna Vinschen wrote: > On Aug 24 15:15, Jon Turney wrote: >> Clicking on a button to cycle around alternatives seems bad UI, so instead allow >> a specific filter view to be directly selected with a pop-up menu. >> >> I'm not sure if this is a great improvement or not, but since I wrote it, here >> it is... > > Nice idea and works fine in my limited testing. > > One question: Usually the popup menu itself reflects the current choice. > So, wouldn't it make more sense to have the text "View" just as plain > text to the left, and the actual popup menu next right to it, showing > the current choice? Yes? No? Shut up? Yeah, there is probably better visual language to be used here. If the popup menu itself reflects the current choice, perhaps we don't need a caption with the current choice at all (i.e. just a button that says "View" and that's all...) I'd really like that button to have a 'â–¾' to indicate it has a drop-down, but that seems hard to achieve. (This might be done with the BS_SPLITBUTTON style (which combines a button and dropdown menu), but that is sadly only supported by common controls on Vista or later.) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-25 19:11 ` Jon Turney @ 2016-08-25 20:34 ` Yaakov Selkowitz 2016-08-26 8:07 ` Corinna Vinschen 0 siblings, 1 reply; 17+ messages in thread From: Yaakov Selkowitz @ 2016-08-25 20:34 UTC (permalink / raw) To: cygwin-apps On 2016-08-25 14:11, Jon Turney wrote: > On 24/08/2016 17:50, Corinna Vinschen wrote: >> One question: Usually the popup menu itself reflects the current choice. >> So, wouldn't it make more sense to have the text "View" just as plain >> text to the left, and the actual popup menu next right to it, showing >> the current choice? Yes? No? Shut up? > > Yeah, there is probably better visual language to be used here. > > If the popup menu itself reflects the current choice, perhaps we don't > need a caption with the current choice at all (i.e. just a button that > says "View" and that's all...) > > I'd really like that button to have a 'â–¾' to indicate it has a > drop-down, but that seems hard to achieve. > > (This might be done with the BS_SPLITBUTTON style (which combines a > button and dropdown menu), but that is sadly only supported by common > controls on Vista or later.) We're dropping Cygwin support for XP anyway, so why not from the installer as well? -- Yaakov ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-25 20:34 ` Yaakov Selkowitz @ 2016-08-26 8:07 ` Corinna Vinschen 2016-08-26 14:41 ` Jon Turney 0 siblings, 1 reply; 17+ messages in thread From: Corinna Vinschen @ 2016-08-26 8:07 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 1262 bytes --] On Aug 25 15:34, Yaakov Selkowitz wrote: > On 2016-08-25 14:11, Jon Turney wrote: > > On 24/08/2016 17:50, Corinna Vinschen wrote: > > > One question: Usually the popup menu itself reflects the current choice. > > > So, wouldn't it make more sense to have the text "View" just as plain > > > text to the left, and the actual popup menu next right to it, showing > > > the current choice? Yes? No? Shut up? > > > > Yeah, there is probably better visual language to be used here. > > > > If the popup menu itself reflects the current choice, perhaps we don't > > need a caption with the current choice at all (i.e. just a button that > > says "View" and that's all...) > > > > I'd really like that button to have a '▾' to indicate it has a > > drop-down, but that seems hard to achieve. > > > > (This might be done with the BS_SPLITBUTTON style (which combines a > > button and dropdown menu), but that is sadly only supported by common > > controls on Vista or later.) > > We're dropping Cygwin support for XP anyway, so why not from the installer > as well? ACK. Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-26 8:07 ` Corinna Vinschen @ 2016-08-26 14:41 ` Jon Turney 2016-08-26 14:43 ` [PATCH setup 5/6] Simplify view mode indication Jon Turney 2016-08-26 15:25 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 0 siblings, 2 replies; 17+ messages in thread From: Jon Turney @ 2016-08-26 14:41 UTC (permalink / raw) To: cygwin-apps On 26/08/2016 09:07, Corinna Vinschen wrote: > On Aug 25 15:34, Yaakov Selkowitz wrote: >> On 2016-08-25 14:11, Jon Turney wrote: >>> On 24/08/2016 17:50, Corinna Vinschen wrote: >>>> One question: Usually the popup menu itself reflects the current choice. >>>> So, wouldn't it make more sense to have the text "View" just as plain >>>> text to the left, and the actual popup menu next right to it, showing >>>> the current choice? Yes? No? Shut up? >>> >>> Yeah, there is probably better visual language to be used here. >>> >>> If the popup menu itself reflects the current choice, perhaps we don't >>> need a caption with the current choice at all (i.e. just a button that >>> says "View" and that's all...) >>> >>> I'd really like that button to have a 'â–¾' to indicate it has a >>> drop-down, but that seems hard to achieve. >>> >>> (This might be done with the BS_SPLITBUTTON style (which combines a >>> button and dropdown menu), but that is sadly only supported by common >>> controls on Vista or later.) Two more patches follow this mail, which do that. I'm still not totally happy with this. It could give the impression that something is supposed to happen when the "View" part of the the button is clicked, and there is no keyboard shortcut to open the view selection menu. >> We're dropping Cygwin support for XP anyway, so why not from the installer >> as well? > > ACK. I guess so, if a splitbutton is the right solution. Should this be done using ld flags --major-os-version 6 --minor-os-version 0? Or some sort of runtime check? ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 5/6] Simplify view mode indication 2016-08-26 14:41 ` Jon Turney @ 2016-08-26 14:43 ` Jon Turney 2016-08-26 14:43 ` [PATCH setup 6/6] Use a splitbutton to show the view choosing popup menu Jon Turney 2016-08-26 15:25 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 1 sibling, 1 reply; 17+ messages in thread From: Jon Turney @ 2016-08-26 14:43 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney We don't really need a bit of text saying what the current view mode is, when it's indicated in the pop-up menu, so remove that control. This makes PickView::mode_caption() unused, so remove that as well. --- PickView.cc | 22 ---------------------- PickView.h | 1 - choose.cc | 8 -------- res.rc | 6 +----- resource.h | 1 - 5 files changed, 1 insertion(+), 37 deletions(-) diff --git a/PickView.cc b/PickView.cc index 3702b92..b079b08 100644 --- a/PickView.cc +++ b/PickView.cc @@ -223,28 +223,6 @@ PickView::getViewMode () return view_mode; } -const char * -PickView::mode_caption () -{ - switch (view_mode) - { - case views::PackageFull: - return "Full"; - case views::PackagePending: - return "Pending"; - case views::PackageKeeps: - return "Up To Date"; - case views::PackageSkips: - return "Not Installed"; - case views::PackageUserPicked: - return "Picked"; - case views::Category: - return "Category"; - default: - return ""; - } -} - /* meant to be called on packagemeta::categories */ bool isObsolete (set <std::string, casecompare_lt_op> &categories) diff --git a/PickView.h b/PickView.h index 332383a..fc4fbad 100644 --- a/PickView.h +++ b/PickView.h @@ -57,7 +57,6 @@ public: PickView (Category & cat); void init(views _mode); ~PickView(); - const char *mode_caption (); void setObsolete (bool doit); void insert_pkg (packagemeta &); void insert_category (Category *, bool); diff --git a/choose.cc b/choose.cc index 180c788..19c1fc9 100644 --- a/choose.cc +++ b/choose.cc @@ -85,7 +85,6 @@ static ControlAdjuster::ControlInfo ChooserControlsInfo[] = { {IDC_CHOOSE_EXP, CP_RIGHT, CP_TOP}, {IDC_CHOOSE_VIEW, CP_LEFT, CP_TOP}, {IDC_LISTVIEW_POS, CP_RIGHT, CP_TOP}, - {IDC_CHOOSE_VIEWCAPTION, CP_LEFT, CP_TOP}, {IDC_CHOOSE_LIST, CP_STRETCH, CP_STRETCH}, {IDC_CHOOSE_HIDE, CP_LEFT, CP_BOTTOM}, {0, CP_LEFT, CP_TOP} @@ -153,9 +152,6 @@ ChooserPage::createListview () chooser->Show(SW_SHOW); chooser->setViewMode (UpgradeAlsoOption || hasManualSelections ? PickView::views::PackagePending : PickView::views::Category); - if (!SetDlgItemText (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) - Log (LOG_BABBLE) << "Failed to set View button caption %ld" << - GetLastError () << endLog; /* FIXME: do we need to init the desired fields ? */ static int ta[] = { IDC_CHOOSE_KEEP, IDC_CHOOSE_CURR, IDC_CHOOSE_EXP, 0 }; @@ -534,10 +530,6 @@ ChooserPage::selectView (void) } chooser->setViewMode (view_mode); - if (!SetDlgItemText - (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) - Log (LOG_BABBLE) << "Failed to set View button caption " << - GetLastError () << endLog; } INT_PTR CALLBACK diff --git a/res.rc b/res.rc index 10ec375..0d124a6 100644 --- a/res.rc +++ b/res.rc @@ -319,9 +319,7 @@ END // Left-aligned controls. #define SETUP_VIEW_X (7) #define SETUP_VIEW_W (26) -#define SETUP_VIEWCAP_X (SETUP_VIEW_X + SETUP_VIEW_W +5) -#define SETUP_VIEWCAP_W (40) -#define SETUP_SEARCH_X (SETUP_VIEWCAP_X + SETUP_VIEWCAP_W + 80) +#define SETUP_SEARCH_X (SETUP_VIEW_X + SETUP_VIEW_W + 125) #define SETUP_SEARCH_W (32) #define SETUP_SEARCHTEXT_X (SETUP_SEARCH_X + SETUP_SEARCH_W + 2) #define SETUP_SEARCHTEXT_W (60) @@ -336,8 +334,6 @@ FONT 8, "MS Shell Dlg" BEGIN PUSHBUTTON "&View", IDC_CHOOSE_VIEW, SETUP_VIEW_X, 30, SETUP_VIEW_W, 14, WS_EX_RIGHT - LTEXT "", IDC_CHOOSE_VIEWCAPTION, SETUP_VIEWCAP_X, 33, - SETUP_VIEWCAP_W, 10 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, diff --git a/resource.h b/resource.h index e2312ac..91b5b52 100644 --- a/resource.h +++ b/resource.h @@ -132,7 +132,6 @@ #define IDC_DLS_PPROGRESS_TEXT 544 #define IDC_DLS_IPROGRESS_TEXT 545 #define IDC_CHOOSE_INST_TEXT 546 -#define IDC_CHOOSE_VIEWCAPTION 547 #define IDC_CHOOSE_LISTHEADER 548 #define IDC_INS_BL_PACKAGE 549 #define IDC_INS_BL_TOTAL 550 -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 6/6] Use a splitbutton to show the view choosing popup menu 2016-08-26 14:43 ` [PATCH setup 5/6] Simplify view mode indication Jon Turney @ 2016-08-26 14:43 ` Jon Turney 0 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-26 14:43 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Pass WM_NOTIFY notifications for child controls of the property page through to OnMessageCommand as well. Future-proof RootPage::OnMessageCmd() to check the notification code. Give IDC_CHOOSE_VIEW button the BS_SPLITBUTTON style, and handle the BCN_DROPDOWN notification from that to show the view choosing menu. --- choose.cc | 34 +++++++++++++++++++--------------- proppage.cc | 10 +++++++--- res.rc | 6 +++--- root.cc | 3 +++ 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/choose.cc b/choose.cc index 19c1fc9..ecfd5f1 100644 --- a/choose.cc +++ b/choose.cc @@ -385,17 +385,18 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) Log (LOG_BABBLE) << "OnMesageCmd " << id << " " << hwndctl << " " << code << endLog; #endif - if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT) + if (code == EN_CHANGE) { - SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL); - return true; - } - else if (code != BN_CLICKED && code != EN_CHANGE) - { - // Not a click notification, we don't care. - return false; + if (id == IDC_CHOOSE_SEARCH_EDIT) + { + SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL); + return true; + } + else + return false; } - + else if (code == BN_CLICKED) + { switch (id) { case IDC_CHOOSE_CLEAR_SEARCH: @@ -422,10 +423,6 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) changeTrust (TRUST_TEST); break; - case IDC_CHOOSE_VIEW: - selectView(); - break; - case IDC_CHOOSE_HIDE: chooser->setObsolete (!IsButtonChecked (id)); break; @@ -433,9 +430,16 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) // Wasn't recognized or handled. return false; } - - // Was handled since we never got to default above. return true; + } + else if (code == BCN_DROPDOWN) + { + if (id == IDC_CHOOSE_VIEW) + selectView(); + } + + // we don't care. + return false; } static void diff --git a/proppage.cc b/proppage.cc index c03e5f7..31f439f 100644 --- a/proppage.cc +++ b/proppage.cc @@ -139,7 +139,9 @@ PropertyPage::DialogProc (UINT message, WPARAM wParam, LPARAM lParam) return TRUE; } case WM_NOTIFY: - switch (((NMHDR FAR *) lParam)->code) + { + NMHDR *pNmHdr = (NMHDR *) lParam; + switch (pNmHdr->code) { case PSN_APPLY: { @@ -256,10 +258,12 @@ PropertyPage::DialogProc (UINT message, WPARAM wParam, LPARAM lParam) } default: { - // Unrecognized notification - return FALSE; + // Pass unrecognized notifications to WM_COMMAND handler + return OnMessageCmd (pNmHdr->idFrom, pNmHdr->hwndFrom, + pNmHdr->code); } } + } break; case WM_COMMAND: { diff --git a/res.rc b/res.rc index 0d124a6..2cdacfb 100644 --- a/res.rc +++ b/res.rc @@ -318,8 +318,8 @@ END // Left-aligned controls. #define SETUP_VIEW_X (7) -#define SETUP_VIEW_W (26) -#define SETUP_SEARCH_X (SETUP_VIEW_X + SETUP_VIEW_W + 125) +#define SETUP_VIEW_W (33) +#define SETUP_SEARCH_X (SETUP_VIEW_X + SETUP_VIEW_W + 2) #define SETUP_SEARCH_W (32) #define SETUP_SEARCHTEXT_X (SETUP_SEARCH_X + SETUP_SEARCH_W + 2) #define SETUP_SEARCHTEXT_W (60) @@ -333,7 +333,7 @@ CAPTION "Cygwin Setup - Select Packages" FONT 8, "MS Shell Dlg" BEGIN PUSHBUTTON "&View", IDC_CHOOSE_VIEW, SETUP_VIEW_X, 30, SETUP_VIEW_W, - 14, WS_EX_RIGHT + 14, BS_SPLITBUTTON 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, diff --git a/root.cc b/root.cc index edf7a91..80f3162 100644 --- a/root.cc +++ b/root.cc @@ -227,6 +227,9 @@ directory_contains_wrong_version (HWND h) bool RootPage::OnMessageCmd (int id, HWND hwndctl, UINT code) { + if ((code != BN_CLICKED) && (code != EN_CHANGE)) + return false; + switch (id) { -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-26 14:41 ` Jon Turney 2016-08-26 14:43 ` [PATCH setup 5/6] Simplify view mode indication Jon Turney @ 2016-08-26 15:25 ` Corinna Vinschen 2016-08-29 10:03 ` Jon Turney 1 sibling, 1 reply; 17+ messages in thread From: Corinna Vinschen @ 2016-08-26 15:25 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 2195 bytes --] On Aug 26 15:41, Jon Turney wrote: > On 26/08/2016 09:07, Corinna Vinschen wrote: > > On Aug 25 15:34, Yaakov Selkowitz wrote: > > > On 2016-08-25 14:11, Jon Turney wrote: > > > > On 24/08/2016 17:50, Corinna Vinschen wrote: > > > > > One question: Usually the popup menu itself reflects the current choice. > > > > > So, wouldn't it make more sense to have the text "View" just as plain > > > > > text to the left, and the actual popup menu next right to it, showing > > > > > the current choice? Yes? No? Shut up? > > > > > > > > Yeah, there is probably better visual language to be used here. > > > > > > > > If the popup menu itself reflects the current choice, perhaps we don't > > > > need a caption with the current choice at all (i.e. just a button that > > > > says "View" and that's all...) > > > > > > > > I'd really like that button to have a '▾' to indicate it has a > > > > drop-down, but that seems hard to achieve. > > > > > > > > (This might be done with the BS_SPLITBUTTON style (which combines a > > > > button and dropdown menu), but that is sadly only supported by common > > > > controls on Vista or later.) > > Two more patches follow this mail, which do that. > > I'm still not totally happy with this. It could give the impression that > something is supposed to happen when the "View" part of the the button is > clicked, and there is no keyboard shortcut to open the view selection menu. Looks like I didn't explain well enough what I meant. What I had in mind was a pull-down menu just as you did, but which reflects the current state in its text. The word "View" would just show up in fixed text field left of it. I. e.: +--------------------+ View: | Category | v | +--------------------+ After the user chooses "Pending": +--------------------+ View: | Pending | v | +--------------------+ I think this is what most users expect from this pull-down menu style, isn't it? Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 0/4] Use a pop-up menu to select chooser view filter 2016-08-26 15:25 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen @ 2016-08-29 10:03 ` Jon Turney 2016-08-29 10:04 ` [PATCH setup 4/4] Use a drop-down list to directly " Jon Turney 0 siblings, 1 reply; 17+ messages in thread From: Jon Turney @ 2016-08-29 10:03 UTC (permalink / raw) To: cygwin-apps On 26/08/2016 16:25, Corinna Vinschen wrote: > On Aug 26 15:41, Jon Turney wrote: >> On 26/08/2016 09:07, Corinna Vinschen wrote: >>> On Aug 25 15:34, Yaakov Selkowitz wrote: >>>> On 2016-08-25 14:11, Jon Turney wrote: >>>>> On 24/08/2016 17:50, Corinna Vinschen wrote: >>>>>> One question: Usually the popup menu itself reflects the current choice. >>>>>> So, wouldn't it make more sense to have the text "View" just as plain >>>>>> text to the left, and the actual popup menu next right to it, showing >>>>>> the current choice? Yes? No? Shut up? >>>>> >>>>> Yeah, there is probably better visual language to be used here. >>>>> >>>>> If the popup menu itself reflects the current choice, perhaps we don't >>>>> need a caption with the current choice at all (i.e. just a button that >>>>> says "View" and that's all...) >>>>> >>>>> I'd really like that button to have a 'â–¾' to indicate it has a >>>>> drop-down, but that seems hard to achieve. >>>>> >>>>> (This might be done with the BS_SPLITBUTTON style (which combines a >>>>> button and dropdown menu), but that is sadly only supported by common >>>>> controls on Vista or later.) >> >> Two more patches follow this mail, which do that. >> >> I'm still not totally happy with this. It could give the impression that >> something is supposed to happen when the "View" part of the the button is >> clicked, and there is no keyboard shortcut to open the view selection menu. > > Looks like I didn't explain well enough what I meant. What I had in > mind was a pull-down menu just as you did, but which reflects the > current state in its text. The word "View" would just show up in fixed > text field left of it. I. e.: > > +--------------------+ > View: | Category | v | > +--------------------+ > > After the user chooses "Pending": > > +--------------------+ > View: | Pending | v | > +--------------------+ > > I think this is what most users expect from this pull-down menu style, > isn't it? Ah, right. Using a drop-down list is the correct thing to do here. I had the idea in the back of my mind that in future it might be worthwhile to add other actions here (e.g. expand/collapse all in category view, unpacking the fixed filter set we support into the tests they are composed of, etc.), where a menu would be more appropriate, but I guess we can cross that bridge if we get to it... See the replacement [4/4] to follow this mail. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH setup 4/4] Use a drop-down list to directly select chooser view filter 2016-08-29 10:03 ` Jon Turney @ 2016-08-29 10:04 ` Jon Turney 2016-08-30 13:01 ` Corinna Vinschen 0 siblings, 1 reply; 17+ messages in thread From: Jon Turney @ 2016-08-29 10:04 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney Rather than a button for cycling through views, use a drop-down list to choose the view Remove not very useful PickView::views::Unknown enum value so that enum can start from 0, so it can be used directly as a drop-down list index. Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk> --- PickView.cc | 28 +++++++++++----------------- PickView.h | 7 +++---- choose.cc | 52 ++++++++++++++++++++++++++++++++++------------------ choose.h | 1 + res.rc | 20 ++++++++++---------- 5 files changed, 59 insertions(+), 49 deletions(-) diff --git a/PickView.cc b/PickView.cc index 25d43a2..fc6f8c2 100644 --- a/PickView.cc +++ b/PickView.cc @@ -81,11 +81,9 @@ DoInsertItem (HWND hwndHeader, int iInsertAfter, int nWidth, LPSTR lpsz) int PickView::set_header_column_order (views vm) { - if (vm == views::Unknown) - return -1; - else if (vm == views::PackageFull || vm == views::PackagePending - || vm == views::PackageKeeps || vm == views::PackageSkips - || vm == views::PackageUserPicked) + if (vm == views::PackageFull || vm == views::PackagePending + || vm == views::PackageKeeps || vm == views::PackageSkips + || vm == views::PackageUserPicked) { headers = pkg_headers; current_col = 0; @@ -141,16 +139,6 @@ PickView::note_width (PickView::Header *hdrs, HDC dc, } void -PickView::cycleViewMode () -{ - PickView::views _value = (PickView::views)((int)view_mode + 1); - if (_value > PickView::views::Category) - _value = PickView::views::PackageFull; - - setViewMode (_value); -} - -void PickView::setViewMode (views mode) { view_mode = mode; @@ -227,10 +215,16 @@ PickView::setViewMode (views mode) InvalidateRect (GetHWND(), &r, TRUE); } +PickView::views +PickView::getViewMode () +{ + return view_mode; +} + const char * -PickView::mode_caption () +PickView::mode_caption (views mode) { - switch (view_mode) + switch (mode) { case views::PackageFull: return "Full"; diff --git a/PickView.h b/PickView.h index c07249b..298f844 100644 --- a/PickView.h +++ b/PickView.h @@ -44,8 +44,8 @@ public: class Header; int num_columns; void defaultTrust (trusts trust); - void cycleViewMode (); void setViewMode (views mode); + views getViewMode (); void DrawIcon (HDC hdc, int x, int y, HANDLE hIcon); void paint (HWND hwnd); LRESULT CALLBACK list_click (HWND hwnd, BOOL dblclk, int x, int y, UINT hitCode); @@ -57,7 +57,7 @@ public: PickView (Category & cat); void init(views _mode); ~PickView(); - const char *mode_caption (); + static const char *mode_caption (views mode); void setObsolete (bool doit); void insert_pkg (packagemeta &); void insert_category (Category *, bool); @@ -98,8 +98,7 @@ public: enum class views { - Unknown, - PackageFull, + PackageFull = 0, PackagePending, PackageKeeps, PackageSkips, diff --git a/choose.cc b/choose.cc index 3c7f4f8..687addf 100644 --- a/choose.cc +++ b/choose.cc @@ -153,9 +153,7 @@ ChooserPage::createListview () chooser->Show(SW_SHOW); chooser->setViewMode (UpgradeAlsoOption || hasManualSelections ? PickView::views::PackagePending : PickView::views::Category); - if (!SetDlgItemText (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) - Log (LOG_BABBLE) << "Failed to set View button caption %ld" << - GetLastError () << endLog; + SendMessage (GetDlgItem (IDC_CHOOSE_VIEW), CB_SETCURSEL, (WPARAM)chooser->getViewMode(), 0); /* FIXME: do we need to init the desired fields ? */ static int ta[] = { IDC_CHOOSE_KEEP, IDC_CHOOSE_CURR, IDC_CHOOSE_EXP, 0 }; @@ -241,6 +239,16 @@ ChooserPage::OnInit () { CheckDlgButton (GetHWND (), IDC_CHOOSE_HIDE, BST_CHECKED); + /* Populate view dropdown list with choices */ + HWND viewlist = GetDlgItem (IDC_CHOOSE_VIEW); + SendMessage (viewlist, CB_RESETCONTENT, 0, 0); + for (int view = (int)PickView::views::PackageFull; + view <= (int)PickView::views::Category; + view++) + { + SendMessage(viewlist, CB_ADDSTRING, 0, (LPARAM)PickView::mode_caption((PickView::views)view)); + } + SetBusy (); if (source == IDC_SOURCE_DOWNLOAD || source == IDC_SOURCE_LOCALDIR) packagemeta::ScanDownloadedFiles (MirrorOption); @@ -385,17 +393,17 @@ ChooserPage::changeTrust(trusts aTrust) bool ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) { +#if DEBUG + Log (LOG_BABBLE) << "OnMesageCmd " << id << " " << hwndctl << " " << code << endLog; +#endif + if (code == EN_CHANGE && id == IDC_CHOOSE_SEARCH_EDIT) { SetTimer(GetHWND (), timer_id, SEARCH_TIMER_DELAY, (TIMERPROC) NULL); return true; } - else if (code != BN_CLICKED && code != EN_CHANGE) - { - // Not a click notification, we don't care. - return false; - } - + else if (code == BN_CLICKED) + { switch (id) { case IDC_CHOOSE_CLEAR_SEARCH: @@ -422,14 +430,6 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) changeTrust (TRUST_TEST); break; - case IDC_CHOOSE_VIEW: - chooser->cycleViewMode (); - if (!SetDlgItemText - (GetHWND (), IDC_CHOOSE_VIEWCAPTION, chooser->mode_caption ())) - Log (LOG_BABBLE) << "Failed to set View button caption " << - GetLastError () << endLog; - break; - case IDC_CHOOSE_HIDE: chooser->setObsolete (!IsButtonChecked (id)); break; @@ -437,9 +437,25 @@ ChooserPage::OnMessageCmd (int id, HWND hwndctl, UINT code) // Wasn't recognized or handled. return false; } - // Was handled since we never got to default above. return true; + } + else if (code == CBN_SELCHANGE) + { + if (id == IDC_CHOOSE_VIEW) + { + // switch to the selected view + LRESULT view_mode = SendMessage (GetDlgItem (IDC_CHOOSE_VIEW), + CB_GETCURSEL, 0, 0); + if (view_mode != CB_ERR) + chooser->setViewMode ((PickView::views)view_mode); + + return true; + } + } + + // We don't care. + return false; } INT_PTR CALLBACK diff --git a/choose.h b/choose.h index 46f0f35..8832f5e 100644 --- a/choose.h +++ b/choose.h @@ -59,6 +59,7 @@ private: void logResults(); void setPrompt(char const *aPrompt); void PlaceDialog (bool); + void selectView (void); PickView *chooser; static HWND ins_dialog; diff --git a/res.rc b/res.rc index 2fae133..3ddf751 100644 --- a/res.rc +++ b/res.rc @@ -318,10 +318,10 @@ END // Left-aligned controls. #define SETUP_VIEW_X (7) -#define SETUP_VIEW_W (26) -#define SETUP_VIEWCAP_X (SETUP_VIEW_X + SETUP_VIEW_W +5) -#define SETUP_VIEWCAP_W (40) -#define SETUP_SEARCH_X (SETUP_VIEWCAP_X + SETUP_VIEWCAP_W + 80) +#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_W (32) #define SETUP_SEARCHTEXT_X (SETUP_SEARCH_X + SETUP_SEARCH_W + 2) #define SETUP_SEARCHTEXT_W (60) @@ -334,10 +334,10 @@ STYLE DS_MODALFRAME | DS_3DLOOK | WS_CHILD | WS_VISIBLE | WS_CAPTION | CAPTION "Cygwin Setup - Select Packages" FONT 8, "MS Shell Dlg" BEGIN - PUSHBUTTON "&View", IDC_CHOOSE_VIEW, SETUP_VIEW_X, 30, SETUP_VIEW_W, - 14, WS_EX_RIGHT - LTEXT "", IDC_CHOOSE_VIEWCAPTION, SETUP_VIEWCAP_X, 33, - SETUP_VIEWCAP_W, 10 + LTEXT "View:", IDC_CHOOSE_VIEWCAPTION, SETUP_VIEW_X, 33, + SETUP_VIEW_W, 10 + COMBOBOX IDC_CHOOSE_VIEW, SETUP_VIEWLIST_X, 30, SETUP_VIEWLIST_W, 84, + CBS_DROPDOWNLIST 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, @@ -536,8 +536,8 @@ BEGIN "considered the most stable. (RECOMMENDED)" IDS_TRUSTEXP_TOOLTIP "Globally select the most recent version, even if " "that version is considered Experimental or for test use by the maintainer." - IDS_VIEWBUTTON_TOOLTIP "Cycles the package view. This determines " - "which packages are shown in the chooser below.\r\n" + IDS_VIEWBUTTON_TOOLTIP "Select the package view. This determines " + "which packages are shown below.\r\n" "\r\n" "Category: Group by package category. Click on '+' to expand.\r\n" "\r\n" -- 2.8.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 4/4] Use a drop-down list to directly select chooser view filter 2016-08-29 10:04 ` [PATCH setup 4/4] Use a drop-down list to directly " Jon Turney @ 2016-08-30 13:01 ` Corinna Vinschen 2016-08-30 23:29 ` Jon Turney 0 siblings, 1 reply; 17+ messages in thread From: Corinna Vinschen @ 2016-08-30 13:01 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 873 bytes --] On Aug 29 11:04, Jon Turney wrote: > Rather than a button for cycling through views, use a drop-down list to > choose the view > > Remove not very useful PickView::views::Unknown enum value so that enum can > start from 0, so it can be used directly as a drop-down list index. Almost Perfect. Please apply, without the colon after "View" (compare with "Search"). Btw. What still bugs me is that the static text elements do not vertically align with the texts in the input elements. I. e., the word "View" is a dot or two lower than, say, the word "Category" in the drop down menu. Same for "Search" and the input field. Can we fix this easily? Any such patch is pre-approved. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH setup 4/4] Use a drop-down list to directly select chooser view filter 2016-08-30 13:01 ` Corinna Vinschen @ 2016-08-30 23:29 ` Jon Turney 0 siblings, 0 replies; 17+ messages in thread From: Jon Turney @ 2016-08-30 23:29 UTC (permalink / raw) To: cygwin-apps On 30/08/2016 14:01, Corinna Vinschen wrote: > On Aug 29 11:04, Jon Turney wrote: >> Rather than a button for cycling through views, use a drop-down list to >> choose the view >> >> Remove not very useful PickView::views::Unknown enum value so that enum can >> start from 0, so it can be used directly as a drop-down list index. > > Almost Perfect. Please apply, without the colon after "View" (compare > with "Search"). Done. > What still bugs me is that the static text elements do not vertically > align with the texts in the input elements. I. e., the word "View" is a > dot or two lower than, say, the word "Category" in the drop down menu. > Same for "Search" and the input field. Can we fix this easily? Any > such patch is pre-approved. I don't quite understand why the text doesn't look centered on the control at the moment. The controls are the standard 14 DLUs high, 1 vertical DLU = 1/8th of the average character height, and (14 - 8)/2 = 3 DLUs is used as the offset between the top of the control and the top of the label. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-08-30 23:29 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-24 14:16 [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Jon Turney 2016-08-24 14:16 ` [PATCH setup 4/4] Use a pop-up menu to directly " Jon Turney 2016-08-24 14:16 ` [PATCH setup 2/4] Change PickView::view into an enum Jon Turney 2016-08-24 14:16 ` [PATCH setup 3/4] Rename PickView::Package to PickView::PackagePending Jon Turney 2016-08-24 14:16 ` [PATCH setup 1/4] Build C++ code with -std=gnu++11 Jon Turney 2016-08-24 16:50 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 2016-08-25 19:11 ` Jon Turney 2016-08-25 20:34 ` Yaakov Selkowitz 2016-08-26 8:07 ` Corinna Vinschen 2016-08-26 14:41 ` Jon Turney 2016-08-26 14:43 ` [PATCH setup 5/6] Simplify view mode indication Jon Turney 2016-08-26 14:43 ` [PATCH setup 6/6] Use a splitbutton to show the view choosing popup menu Jon Turney 2016-08-26 15:25 ` [PATCH setup 0/4] Use a pop-up menu to select chooser view filter Corinna Vinschen 2016-08-29 10:03 ` Jon Turney 2016-08-29 10:04 ` [PATCH setup 4/4] Use a drop-down list to directly " Jon Turney 2016-08-30 13:01 ` Corinna Vinschen 2016-08-30 23:29 ` Jon Turney
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).