On Wed Dec 28 19:30:30 2022 +0000, Esme Povirk wrote:
> I think removing items from the list like this is confusing. It also
> results in the need for a special case "preserving" the first crumb in
> the layout code.
This is similar to what Windows does, but it indeed does complicate things a little, I'll remove it.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1857#note_20180
Esme Povirk (@madewokherd) commented about dlls/comdlg32/navbar.c:
> hwnd, 0, COMDLG32_hInstance, NULL);
> SetWindowSubclass(info->background_hwnd, NAVBAR_BACKGROUND_SubclassProc, 0, (DWORD_PTR)info);
>
> + info->overflow_hwnd = CreateWindowExW(0, WC_BUTTONW, NULL,
For accessibility it'd be good to give this button a title.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1857#note_20177
Esme Povirk (@madewokherd) commented about dlls/comdlg32/navbar.c:
> while (ILRemoveLastID(pidl));
> ILFree(pidl);
>
> + NAVBAR_OVERFLOW_Clear(info);
> +
> + /* move the first crumb to overflow menu, if there are at least 2 crumbs */
> + crumb1 = LIST_ENTRY(list_head(&new_crumbs), struct crumb, entry);
> + if (crumb1 && crumb1->entry.next != &new_crumbs)
> + {
> + NAVBAR_OVERFLOW_Insert(info, crumb1->pidl, crumb1->display_name);
> + DestroyWindow(crumb1->hwnd);
> +
> + list_remove(&crumb1->entry);
I think removing items from the list like this is confusing. It also results in the need for a special case "preserving" the first crumb in the layout code.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1857#note_20178
Esme Povirk (@madewokherd) commented about dlls/comdlg32/navbar.c:
> + menu_item.hbmpItem = HBMMENU_CALLBACK; /* see NAVBAR_OVERFLOW_DrawIcon */
> +
> + InsertMenuItemW(info->overflow_menu, -1, TRUE, &menu_item);
> +}
> +
> +static void NAVBAR_OVERFLOW_Clear(NAVBAR_INFO *info)
> +{
> + INT i, menu_item_count = GetMenuItemCount(info->overflow_menu);
> + MENUITEMINFOW menu_item = {0};
> +
> + menu_item.cbSize = sizeof(MENUITEMINFOW);
> + menu_item.fMask = MIIM_DATA;
> +
> + for (i = 0; i < menu_item_count; i++)
> + {
> + /* deleting an item shifts further indices, so don't use `i` */
It would probably be simpler to delete the items from the end, and slightly more efficient because it removes the need to shift indices.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/1857#note_20175