From ea0f5144c9da7bcd5174dd068f9e949e7f9bc4ed Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Sat, 5 Apr 2025 16:33:46 +0200 Subject: [PATCH] title_bar: Ensure git onboarding banner dismissal is properly respected (#28147) A user reported this issue [on Discord](https://discord.com/channels/869392257814519848/873292398204170290/1357879959422636185). The issue here only arises for users which recently installed Zed or had previously not dismissed the Git Onboarding component. It was introduced by https://github.com/zed-industries/zed/pull/27412, which made the banner component reusable. For every banner, there is a value stored in the KVP store when it was first dismissed. For the git onboarding banner, this was `zed_git_banner_dismissed_at` initially, but this key would have been changed by the linked PR. A change would have resulted in the banner being shown again for users who already dismissed the panel, so for the special case of `Git Onboarding`, a check was added which ensured this would not happen. However, this check was only added for reading from the key from the DB but not on writing the git onboarding dismissal it to the DB. Thus, if a user who had not previously dismissed the panel opened Zed, we would check for the old key to be present in the DB. Since that would not be the case, the banner would be shown. If the user dismissed the panel, it would be stored in the database with the new key. Thus, on a reopen of Zed, the banner would again be shown since for the old key there would still be no value present and users are unable to dismiss the panel. This PR fixes this behavior by moving the check into the method that generates the key. With this, users which were unaffected by the bug will still not see the panel again. Users who would install Zed with this change present will be able to properly dismiss the panel aswell. Users which were affected by the bug need to dismiss the banner one more time. That happens because I did not want to modify the dismissal check to check for two keys (the original one and the new one), as it would clutter the logic even more for this special case. If this would be preferred, feel free to let me know. Release Notes: - Fixed an issue where dismissing the git onboarding banner would not be persisted across sessions. --- crates/title_bar/src/onboarding_banner.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/title_bar/src/onboarding_banner.rs b/crates/title_bar/src/onboarding_banner.rs index 169f555f3f..8ed6e956af 100644 --- a/crates/title_bar/src/onboarding_banner.rs +++ b/crates/title_bar/src/onboarding_banner.rs @@ -59,19 +59,18 @@ impl OnboardingBanner { } fn dismissed_at_key(source: &str) -> String { - format!( - "{}_{}", - "_banner_dismissed_at", - source.to_lowercase().trim().replace(" ", "_") - ) + if source == "Git Onboarding" { + "zed_git_banner_dismissed_at".to_string() + } else { + format!( + "{}_banner_dismissed_at", + source.to_lowercase().trim().replace(" ", "_") + ) + } } fn get_dismissed(source: &str) -> bool { - let dismissed_at = if source == "Git Onboarding" { - "zed_git_banner_dismissed_at".to_string() - } else { - dismissed_at_key(source) - }; + let dismissed_at = dismissed_at_key(source); db::kvp::KEY_VALUE_STORE .read_kvp(&dismissed_at) .log_err()