From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id A57F41FF09B for ; Mon, 31 Aug 2026 15:11:39 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 59C1B213C3; Mon, 31 Aug 2026 15:11:39 +0200 (CEST) Message-ID: <19931a6b-a366-48f2-a48c-119d4380fb92@proxmox.com> Date: Mon, 31 Aug 2026 15:11:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH datacenter-manager/yew-widget-toolkit 0/3] ui: persist the main menus expanded state To: Lukas Wagner , pdm-devel@lists.proxmox.com References: <20260820081803.991511-1-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788181883158 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.597 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: GWZU5RB3DQV7KALQRIB7NBJ4X5E7A2U4 X-Message-ID-Hash: GWZU5RB3DQV7KALQRIB7NBJ4X5E7A2U4 X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 8/31/26 3:02 PM, Lukas Wagner wrote: > On Thu Aug 20, 2026 at 10:17 AM CEST, Dominik Csapak wrote: >> By saving the state in the browsers local storage. >> >> It also fixes an issue where menus that were collapsed by default wouldn't open >> on the first click. >> >> Note that for the pdm patches, pwt must be bumped and the new version must be >> recorded in Cargo.toml. >> >> >> proxmox-yew-widget-toolki: >> >> Dominik Csapak (2): >> widget: navigation drawer: fix first open for default collapsed menus >> widget: navigation drawer: allow persisting the expanded state >> >> src/widget/nav/navigation_drawer.rs | 138 +++++++++++++++++++++++----- >> 1 file changed, 115 insertions(+), 23 deletions(-) >> >> >> proxmox-datacenter-manager: >> >> Dominik Csapak (1): >> ui: main menu: make expanded/collapsed state persistent >> >> ui/src/main_menu.rs | 1 + >> 1 file changed, 1 insertion(+) >> >> >> Summary over all repositories: >> 2 files changed, 116 insertions(+), 23 deletions(-) > > > LGTM in general, works fine and the code seems good too. > > One thought, looking at the saved state in the web inspector in Firefox: > Would it maybe make sense for future extensions to slightly update the > serialized format from > > ["sdn","administration"] > > to something like: > > { collapsed: ["sdn", "administration"]} > > That would make it much easier to add other keys as well in the future, > if we need to store additional state. it's actually not collapsed, but changed from the default (the main menu in pdm just happens to default to all expanded) so it would be { "changed": [...] } would that be ok? we could of course do this in the future and interpret an array as just the changed list and migrate automatically. in practice this would look like (rough pseude code): let try1: Result = input.parse(); match try1 { Ok(state) => { /* return the state */ } Err(err) => { let legacy: Result = input.parse(); // either take legacy and convert to new state // or return original error 'err' } } > > Not a blocker from my side though: > > Reviewed-by: Lukas Wagner > Tested-by: Lukas Wagner