From d0641a38a4f024f0e6a1be3da8e8169ca89aa20e Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Thu, 20 Mar 2025 10:28:43 -0400 Subject: [PATCH] Rework feedback modal (#27186) After our last community sync, we came to the conclusion that feedback being sent outside of email is difficult to reply to. Our decision was to use the old, tried and true email system, so that we can better respond to people asking questions. SCR-20250320-igub Release Notes: - N/A --------- Co-authored-by: Danilo Leal --- Cargo.lock | 12 - crates/feedback/Cargo.toml | 13 - crates/feedback/src/feedback.rs | 28 +- crates/feedback/src/feedback_modal.rs | 557 +++----------------------- 4 files changed, 77 insertions(+), 533 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d993f4ac1f786087ddce6c030526658082cc6061..752feab0b721c1e29fe30863aa5a596f824824f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4927,25 +4927,13 @@ dependencies = [ name = "feedback" version = "0.1.0" dependencies = [ - "anyhow", - "bitflags 2.8.0", "client", - "db", "editor", - "futures 0.3.31", "gpui", - "http_client", "human_bytes", - "language", - "log", "menu", - "project", - "regex", "release_channel", "serde", - "serde_derive", - "serde_json", - "smol", "sysinfo", "ui", "urlencoding", diff --git a/crates/feedback/Cargo.toml b/crates/feedback/Cargo.toml index 05577afc0445f643835e3d6228300ce25f9ed5a0..c479303db748ed44474ed645d63701bbddab97fb 100644 --- a/crates/feedback/Cargo.toml +++ b/crates/feedback/Cargo.toml @@ -15,25 +15,12 @@ path = "src/feedback.rs" test-support = [] [dependencies] -anyhow.workspace = true -bitflags.workspace = true client.workspace = true -db.workspace = true -editor.workspace = true -futures.workspace = true gpui.workspace = true -http_client.workspace = true human_bytes = "0.4.1" -language.workspace = true -log.workspace = true menu.workspace = true -project.workspace = true -regex.workspace = true release_channel.workspace = true serde.workspace = true -serde_derive.workspace = true -serde_json.workspace = true -smol.workspace = true sysinfo.workspace = true ui.workspace = true urlencoding.workspace = true diff --git a/crates/feedback/src/feedback.rs b/crates/feedback/src/feedback.rs index 635f90637bb7819e943357432a4ea4f67e39dd5e..c1a23b42dd916257bf8ed4e7ac64ee3d258076c6 100644 --- a/crates/feedback/src/feedback.rs +++ b/crates/feedback/src/feedback.rs @@ -11,19 +11,18 @@ actions!( zed, [ CopySystemSpecsIntoClipboard, + EmailZed, FileBugReport, + OpenZedRepo, RequestFeature, - OpenZedRepo ] ); -const fn zed_repo_url() -> &'static str { - "https://github.com/zed-industries/zed" -} +const ZED_REPO_URL: &str = "https://github.com/zed-industries/zed"; -fn request_feature_url() -> String { - "https://github.com/zed-industries/zed/discussions/new/choose".to_string() -} +const REQUEST_FEATURE_URL: &str = "https://github.com/zed-industries/zed/discussions/new/choose"; + +const EMAIL_ZED_URL: &str = "mailto:hi@zed.dev"; fn file_bug_report_url(specs: &SystemSpecs) -> String { format!( @@ -66,14 +65,8 @@ pub fn init(cx: &mut App) { }) .detach(); }) - .register_action(|_, _: &RequestFeature, window, cx| { - cx.spawn_in(window, async move |_, cx| { - cx.update(|_, cx| { - cx.open_url(&request_feature_url()); - }) - .log_err(); - }) - .detach(); + .register_action(|_, _: &RequestFeature, _, cx| { + cx.open_url(REQUEST_FEATURE_URL); }) .register_action(move |_, _: &FileBugReport, window, cx| { let specs = SystemSpecs::new(window, cx); @@ -86,8 +79,11 @@ pub fn init(cx: &mut App) { }) .detach(); }) + .register_action(move |_, _: &EmailZed, _, cx| { + cx.open_url(EMAIL_ZED_URL); + }) .register_action(move |_, _: &OpenZedRepo, _, cx| { - cx.open_url(zed_repo_url()); + cx.open_url(ZED_REPO_URL); }); }) .detach(); diff --git a/crates/feedback/src/feedback_modal.rs b/crates/feedback/src/feedback_modal.rs index f3cba09bc12085e2bacee3b26f363a3e83a90e63..45d656c174b6b12194bbdb16486d8db26e5c95db 100644 --- a/crates/feedback/src/feedback_modal.rs +++ b/crates/feedback/src/feedback_modal.rs @@ -1,421 +1,37 @@ -use std::{ - ops::RangeInclusive, - sync::{Arc, LazyLock}, - time::Duration, -}; - -use anyhow::{anyhow, bail}; -use bitflags::bitflags; -use client::Client; -use db::kvp::KEY_VALUE_STORE; -use editor::{Editor, EditorEvent}; -use futures::AsyncReadExt; -use gpui::{ - div, rems, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, - PromptLevel, Render, Task, Window, -}; -use http_client::HttpClient; -use language::Buffer; -use project::Project; -use regex::Regex; -use serde_derive::Serialize; -use ui::{prelude::*, Button, ButtonStyle, IconPosition, Tooltip}; -use util::ResultExt; -use workspace::{DismissDecision, ModalView, Workspace}; +use gpui::{App, Context, DismissEvent, EventEmitter, FocusHandle, Focusable, Render, Window}; +use ui::{prelude::*, IconPosition}; +use workspace::{ModalView, Workspace}; use zed_actions::feedback::GiveFeedback; -use crate::{system_specs::SystemSpecs, OpenZedRepo}; - -// For UI testing purposes -const SEND_SUCCESS_IN_DEV_MODE: bool = true; -const SEND_TIME_IN_DEV_MODE: Duration = Duration::from_secs(2); - -// Temporary, until tests are in place -#[cfg(debug_assertions)] -const DEV_MODE: bool = true; - -#[cfg(not(debug_assertions))] -const DEV_MODE: bool = false; - -const DATABASE_KEY_NAME: &str = "email_address"; -static EMAIL_REGEX: LazyLock = - LazyLock::new(|| Regex::new(r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b").unwrap()); -const FEEDBACK_CHAR_LIMIT: RangeInclusive = 10..=5000; -const FEEDBACK_SUBMISSION_ERROR_TEXT: &str = - "Feedback failed to submit, see error log for details."; - -#[derive(Serialize)] -struct FeedbackRequestBody<'a> { - feedback_text: &'a str, - email: Option, - installation_id: Option>, - metrics_id: Option>, - system_specs: SystemSpecs, - is_staff: bool, -} - -bitflags! { - #[derive(Debug, Clone, PartialEq)] - struct InvalidStateFlags: u8 { - const EmailAddress = 0b00000001; - const CharacterCount = 0b00000010; - } -} - -#[derive(Debug, Clone, PartialEq)] -enum CannotSubmitReason { - InvalidState { flags: InvalidStateFlags }, - AwaitingSubmission, -} - -#[derive(Debug, Clone, PartialEq)] -enum SubmissionState { - CanSubmit, - CannotSubmit { reason: CannotSubmitReason }, -} +use crate::{EmailZed, FileBugReport, OpenZedRepo, RequestFeature}; pub struct FeedbackModal { - system_specs: SystemSpecs, - feedback_editor: Entity, - email_address_editor: Entity, - submission_state: Option, - dismiss_modal: bool, - character_count: i32, + focus_handle: FocusHandle, } impl Focusable for FeedbackModal { - fn focus_handle(&self, cx: &App) -> FocusHandle { - self.feedback_editor.focus_handle(cx) + fn focus_handle(&self, _: &App) -> FocusHandle { + self.focus_handle.clone() } } impl EventEmitter for FeedbackModal {} -impl ModalView for FeedbackModal { - fn on_before_dismiss( - &mut self, - window: &mut Window, - cx: &mut Context, - ) -> DismissDecision { - self.update_email_in_store(window, cx); - - if self.dismiss_modal { - return DismissDecision::Dismiss(true); - } - - let has_feedback = self.feedback_editor.read(cx).text_option(cx).is_some(); - if !has_feedback { - return DismissDecision::Dismiss(true); - } - - let answer = window.prompt( - PromptLevel::Info, - "Discard feedback?", - None, - &["Yes", "No"], - cx, - ); - - cx.spawn_in(window, async move |this, cx| { - if answer.await.ok() == Some(0) { - this.update(cx, |this, cx| { - this.dismiss_modal = true; - cx.emit(DismissEvent) - }) - .log_err(); - } - }) - .detach(); - - DismissDecision::Pending - } -} +impl ModalView for FeedbackModal {} impl FeedbackModal { pub fn register(workspace: &mut Workspace, _: &mut Window, cx: &mut Context) { let _handle = cx.entity().downgrade(); workspace.register_action(move |workspace, _: &GiveFeedback, window, cx| { - workspace - .with_local_workspace(window, cx, |workspace, window, cx| { - let markdown = workspace - .app_state() - .languages - .language_for_name("Markdown"); - - let project = workspace.project().clone(); - - let system_specs = SystemSpecs::new(window, cx); - cx.spawn_in(window, async move |workspace, cx| { - let markdown = markdown.await.log_err(); - let buffer = project.update(cx, |project, cx| { - project.create_local_buffer("", markdown, cx) - })?; - let system_specs = system_specs.await; - - workspace.update_in(cx, |workspace, window, cx| { - workspace.toggle_modal(window, cx, move |window, cx| { - FeedbackModal::new(system_specs, project, buffer, window, cx) - }); - })?; - - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - }) - .detach_and_log_err(cx); + workspace.toggle_modal(window, cx, move |_, cx| FeedbackModal::new(cx)); }); } - pub fn new( - system_specs: SystemSpecs, - project: Entity, - buffer: Entity, - window: &mut Window, - cx: &mut Context, - ) -> Self { - let email_address_editor = cx.new(|cx| { - let mut editor = Editor::single_line(window, cx); - editor.set_placeholder_text("Email address (optional)", cx); - - if let Ok(Some(email_address)) = KEY_VALUE_STORE.read_kvp(DATABASE_KEY_NAME) { - editor.set_text(email_address, window, cx) - } - - editor - }); - - let feedback_editor = cx.new(|cx| { - let mut editor = Editor::for_buffer(buffer, Some(project.clone()), window, cx); - editor.set_placeholder_text( - "You can use markdown to organize your feedback with code and links.", - cx, - ); - editor.set_show_gutter(false, cx); - editor.set_show_indent_guides(false, cx); - editor.set_show_edit_predictions(Some(false), window, cx); - editor.set_vertical_scroll_margin(5, cx); - editor.set_use_modal_editing(false); - editor.set_soft_wrap(); - editor - }); - - cx.subscribe(&feedback_editor, |this, editor, event: &EditorEvent, cx| { - if matches!(event, EditorEvent::Edited { .. }) { - this.character_count = editor - .read(cx) - .buffer() - .read(cx) - .as_singleton() - .expect("Feedback editor is never a multi-buffer") - .read(cx) - .len() as i32; - cx.notify(); - } - }) - .detach(); - + pub fn new(cx: &mut Context) -> Self { Self { - system_specs: system_specs.clone(), - feedback_editor, - email_address_editor, - submission_state: None, - dismiss_modal: false, - character_count: 0, + focus_handle: cx.focus_handle(), } } - pub fn submit( - &mut self, - window: &mut Window, - cx: &mut Context, - ) -> Task> { - let feedback_text = self.feedback_editor.read(cx).text(cx).trim().to_string(); - let email = self.email_address_editor.read(cx).text_option(cx); - - let answer = window.prompt( - PromptLevel::Info, - "Ready to submit your feedback?", - None, - &["Yes, Submit!", "No"], - cx, - ); - let client = Client::global(cx).clone(); - let specs = self.system_specs.clone(); - cx.spawn_in(window, async move |this, cx| { - let answer = answer.await.ok(); - if answer == Some(0) { - this.update(cx, |this, cx| { - this.submission_state = Some(SubmissionState::CannotSubmit { - reason: CannotSubmitReason::AwaitingSubmission, - }); - cx.notify(); - }) - .log_err(); - - let res = - FeedbackModal::submit_feedback(&feedback_text, email, client, specs).await; - - match res { - Ok(_) => { - this.update(cx, |this, cx| { - this.dismiss_modal = true; - cx.notify(); - cx.emit(DismissEvent) - }) - .ok(); - } - Err(error) => { - log::error!("{}", error); - this.update_in(cx, |this, window, cx| { - let prompt = window.prompt( - PromptLevel::Critical, - FEEDBACK_SUBMISSION_ERROR_TEXT, - None, - &["OK"], - cx, - ); - cx.spawn_in(window, async move |_, _cx| { - prompt.await.ok(); - }) - .detach(); - - this.submission_state = Some(SubmissionState::CanSubmit); - cx.notify(); - }) - .log_err(); - } - } - } - }) - .detach(); - - Task::ready(Ok(())) - } - - async fn submit_feedback( - feedback_text: &str, - email: Option, - zed_client: Arc, - system_specs: SystemSpecs, - ) -> anyhow::Result<()> { - if DEV_MODE { - smol::Timer::after(SEND_TIME_IN_DEV_MODE).await; - - if SEND_SUCCESS_IN_DEV_MODE { - return Ok(()); - } else { - return Err(anyhow!("Error submitting feedback")); - } - } - - let telemetry = zed_client.telemetry(); - let installation_id = telemetry.installation_id(); - let metrics_id = telemetry.metrics_id(); - let is_staff = telemetry.is_staff(); - let http_client = zed_client.http_client(); - let feedback_endpoint = http_client.build_url("/api/feedback"); - let request = FeedbackRequestBody { - feedback_text, - email, - installation_id, - metrics_id, - system_specs, - is_staff: is_staff.unwrap_or(false), - }; - let json_bytes = serde_json::to_vec(&request)?; - let request = http_client::http::Request::post(feedback_endpoint) - .header("content-type", "application/json") - .body(json_bytes.into())?; - let mut response = http_client.send(request).await?; - let mut body = String::new(); - response.body_mut().read_to_string(&mut body).await?; - let response_status = response.status(); - if !response_status.is_success() { - bail!("Feedback API failed with error: {}", response_status) - } - Ok(()) - } - - fn update_submission_state(&mut self, cx: &mut Context) { - if self.awaiting_submission() { - return; - } - - let mut invalid_state_flags = InvalidStateFlags::empty(); - - let valid_email_address = match self.email_address_editor.read(cx).text_option(cx) { - Some(email_address) => EMAIL_REGEX.is_match(&email_address), - None => true, - }; - - if !valid_email_address { - invalid_state_flags |= InvalidStateFlags::EmailAddress; - } - - if !FEEDBACK_CHAR_LIMIT.contains(&self.character_count) { - invalid_state_flags |= InvalidStateFlags::CharacterCount; - } - - if invalid_state_flags.is_empty() { - self.submission_state = Some(SubmissionState::CanSubmit); - } else { - self.submission_state = Some(SubmissionState::CannotSubmit { - reason: CannotSubmitReason::InvalidState { - flags: invalid_state_flags, - }, - }); - } - } - - fn update_email_in_store(&self, window: &mut Window, cx: &mut Context) { - let email = self.email_address_editor.read(cx).text_option(cx); - - cx.spawn_in(window, async move |_, _| match email { - Some(email) => { - KEY_VALUE_STORE - .write_kvp(DATABASE_KEY_NAME.to_string(), email) - .await - .ok(); - } - None => { - KEY_VALUE_STORE - .delete_kvp(DATABASE_KEY_NAME.to_string()) - .await - .ok(); - } - }) - .detach(); - } - - fn valid_email_address(&self) -> bool { - !self.in_invalid_state(InvalidStateFlags::EmailAddress) - } - - fn valid_character_count(&self) -> bool { - !self.in_invalid_state(InvalidStateFlags::CharacterCount) - } - - fn in_invalid_state(&self, flag: InvalidStateFlags) -> bool { - match self.submission_state { - Some(SubmissionState::CannotSubmit { - reason: CannotSubmitReason::InvalidState { ref flags }, - }) => flags.contains(flag), - _ => false, - } - } - - fn awaiting_submission(&self) -> bool { - matches!( - self.submission_state, - Some(SubmissionState::CannotSubmit { - reason: CannotSubmitReason::AwaitingSubmission - }) - ) - } - - fn can_submit(&self) -> bool { - matches!(self.submission_state, Some(SubmissionState::CanSubmit)) - } - fn cancel(&mut self, _: &menu::Cancel, _: &mut Window, cx: &mut Context) { cx.emit(DismissEvent) } @@ -423,118 +39,75 @@ impl FeedbackModal { impl Render for FeedbackModal { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - self.update_submission_state(cx); - - let submit_button_text = if self.awaiting_submission() { - "Submitting..." - } else { - "Submit" - }; - let open_zed_repo = cx.listener(|_, _, window, cx| window.dispatch_action(Box::new(OpenZedRepo), cx)); v_flex() - .elevation_3(cx) .key_context("GiveFeedback") .on_action(cx.listener(Self::cancel)) - .min_w(rems(40.)) - .max_w(rems(96.)) - .h(rems(32.)) + .elevation_3(cx) + .w_96() + .h_auto() .p_4() .gap_2() - .child(Headline::new("Give Feedback")) .child( - Label::new(if self.character_count < *FEEDBACK_CHAR_LIMIT.start() { - format!( - "Feedback must be at least {} characters.", - FEEDBACK_CHAR_LIMIT.start() - ) - } else { - format!( - "Characters: {}", - *FEEDBACK_CHAR_LIMIT.end() - self.character_count - ) - }) - .color(if self.valid_character_count() { - Color::Success - } else { - Color::Error - }), + h_flex() + .w_full() + .justify_between() + .child(Headline::new("Give Feedback")) + .child( + IconButton::new("close-btn", IconName::Close) + .icon_color(Color::Muted) + .on_click(cx.listener(move |_, _, window, cx| { + cx.spawn_in(window, async move |this, cx| { + this.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); + }) + .detach(); + })), + ), + ) + .child(Label::new("Thanks for using Zed! To share your experience with us, reach for the channel that's the most appropriate:")) + .child( + Button::new("file-a-bug-report", "File a Bug Report") + .full_width() + .icon(IconName::Debug) + .icon_size(IconSize::XSmall) + .icon_color(Color::Muted) + .icon_position(IconPosition::Start) + .on_click(cx.listener(|_, _, window, cx| { + window.dispatch_action(Box::new(FileBugReport), cx); + })), ) .child( - div() - .flex_1() - .bg(cx.theme().colors().editor_background) - .p_2() - .border_1() - .rounded_sm() - .border_color(cx.theme().colors().border) - .child(self.feedback_editor.clone()), + Button::new("request-a-feature", "Request a Feature") + .full_width() + .icon(IconName::Sparkle) + .icon_size(IconSize::XSmall) + .icon_color(Color::Muted) + .icon_position(IconPosition::Start) + .on_click(cx.listener(|_, _, window, cx| { + window.dispatch_action(Box::new(RequestFeature), cx); + })), ) .child( - v_flex() - .gap_1() - .child( - h_flex() - .bg(cx.theme().colors().editor_background) - .p_2() - .border_1() - .rounded_sm() - .border_color(if self.valid_email_address() { - cx.theme().colors().border - } else { - cx.theme().status().error_border - }) - .child(self.email_address_editor.clone()), - ) - .child( - Label::new("Provide an email address if you want us to be able to reply.") - .size(LabelSize::Small) - .color(Color::Muted), - ), + Button::new("send-us_an-email", "Send an Email") + .full_width() + .icon(IconName::Envelope) + .icon_size(IconSize::XSmall) + .icon_color(Color::Muted) + .icon_position(IconPosition::Start) + .on_click(cx.listener(|_, _, window, cx| { + window.dispatch_action(Box::new(EmailZed), cx); + })), ) .child( - h_flex() - .justify_between() - .gap_1() - .child( - Button::new("zed_repository", "Zed Repository") - .style(ButtonStyle::Transparent) - .icon(IconName::ExternalLink) - .icon_position(IconPosition::End) - .icon_size(IconSize::Small) - .on_click(open_zed_repo), - ) - .child( - h_flex() - .gap_1() - .child( - Button::new("cancel_feedback", "Cancel") - .style(ButtonStyle::Subtle) - .color(Color::Muted) - .on_click(cx.listener(move |_, _, window, cx| { - cx.spawn_in(window, async move |this, cx| { - this.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); - }) - .detach(); - })), - ) - .child( - Button::new("submit_feedback", submit_button_text) - .color(Color::Accent) - .style(ButtonStyle::Filled) - .on_click(cx.listener(|this, _, window, cx| { - this.submit(window, cx).detach(); - })) - .tooltip(move |_, cx| { - Tooltip::simple("Submit feedback to the Zed team.", cx) - }) - .when(!self.can_submit(), |this| this.disabled(true)), - ), - ), + Button::new("zed_repository", "GitHub Repository") + .full_width() + .icon(IconName::Github) + .icon_size(IconSize::XSmall) + .icon_color(Color::Muted) + .icon_position(IconPosition::Start) + .on_click(open_zed_repo), ) } } - -// TODO: Testing of various button states, dismissal prompts, etc. :)