fix: route default topbar feedback button to Typeform#11863
Open
christian-byrne wants to merge 4 commits intomainfrom
Open
fix: route default topbar feedback button to Typeform#11863christian-byrne wants to merge 4 commits intomainfrom
christian-byrne wants to merge 4 commits intomainfrom
Conversation
The default topbar feedback button in WorkflowTabs.vue (shown when Comfy.UI.TabBarLayout != 'Legacy', the default) still called buildFeedbackUrl() and opened Zendesk, so most users never reached the nightly feedback Typeform survey added in #10890. Unify all in-app feedback buttons (default topbar, legacy action bar, Help Center menu) on a single FEEDBACK_TYPEFORM_URL exported from platform/support/config. Drop the now-unused buildFeedbackUrl helper and ZENDESK_FEEDBACK_FORM_ID constant; buildSupportUrl is preserved for the unchanged Comfy.ContactSupport flow.
Add focused unit tests for the two feedback buttons converted in the previous commit: - WorkflowTabs feedback button: opens Typeform on Cloud and Nightly, hidden on other distributions and when the legacy tab bar is active. - Legacy action bar feedback button (cloudFeedbackTopbarButton): registers under Legacy layout and opens Typeform on click. Both tests assert against the shared FEEDBACK_TYPEFORM_URL constant so the URL value cannot drift across the three call sites.
Replace the bare FEEDBACK_TYPEFORM_URL constant with a buildFeedbackTypeformUrl(source) helper that appends: - distribution: ccloud / oss-nightly / oss (preserves the build-tagging that buildFeedbackUrl() previously sent to Zendesk so Cloud, Nightly, and OSS responses stay segmented). - source: topbar / action-bar / help-center (identifies which UI entry point launched the survey). Tags are passed via the URL fragment (Typeform's hidden-field convention), so they reach the survey but are never sent to the server in the request line. Adds a unit test for the builder, a HelpCenterMenuContent test covering the Cloud/Nightly Typeform path and the OSS Comfy.ContactSupport fallback, and updates the existing topbar/action-bar tests to assert the tagged URL.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR replaces the previous Zendesk feedback flow with Typeform. It adds ChangesFeedback Mechanism Migration
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Component (Topbar / HelpCenter / ActionBar)
participant Config as buildFeedbackTypeformUrl
participant Browser as window.open
User->>UI: Click "Give Feedback" / button
UI->>Config: buildFeedbackTypeformUrl(source)
Config->>UI: returns Typeform URL with `#distribution`=...&source=...
alt Cloud or Nightly
UI->>Browser: window.open(URL, target, features)
Browser-->>User: open Typeform in new tab
else OSS
UI->>UI: execute support command (Comfy.ContactSupport)
UI-->>User: show support workflow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 22 minutes and 54 seconds. Comment |
🎭 Playwright: ✅ 1465 passed, 0 failed · 1 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/helpcenter/HelpCenterMenuContent.test.ts (1)
132-133: ⚡ Quick winPrefer accessible queries over test IDs for feedback-item clicks.
On Line 132, Line 146, and Line 159,
getByTestIdworks, butgetByRole('menuitem', { name: ... })would better validate user-facing behavior and accessibility semantics.♻️ Suggested selector update
- await user.click(screen.getByTestId('help-menu-item-feedback')) + await user.click(screen.getByRole('menuitem', { name: 'Feedback' })) - await user.click(screen.getByTestId('help-menu-item-feedback')) + await user.click(screen.getByRole('menuitem', { name: 'Feedback' })) - await user.click(screen.getByTestId('help-menu-item-feedback')) + await user.click(screen.getByRole('menuitem', { name: 'Feedback' }))Based on learnings: "In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes."
Also applies to: 146-147, 159-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/helpcenter/HelpCenterMenuContent.test.ts` around lines 132 - 133, Replace test queries that use getByTestId for help menu items with accessible queries: instead of getByTestId('help-menu-item-feedback') (and the similar 'help-menu-item-request' and 'help-menu-item-docs' usages), use getByRole('menuitem', { name: /Feedback/i }) (and respectively /Request/i, /Docs/i or the exact visible label text) so tests assert on role and accessible name; update the three calls in HelpCenterMenuContent.test.ts to use getByRole('menuitem', { name: ... }) and adjust any click/assert lines to use those returned elements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/helpcenter/HelpCenterMenuContent.test.ts`:
- Around line 132-133: Replace test queries that use getByTestId for help menu
items with accessible queries: instead of getByTestId('help-menu-item-feedback')
(and the similar 'help-menu-item-request' and 'help-menu-item-docs' usages), use
getByRole('menuitem', { name: /Feedback/i }) (and respectively /Request/i,
/Docs/i or the exact visible label text) so tests assert on role and accessible
name; update the three calls in HelpCenterMenuContent.test.ts to use
getByRole('menuitem', { name: ... }) and adjust any click/assert lines to use
those returned elements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97010d7a-91a0-43a2-943f-af45810433af
📒 Files selected for processing (8)
src/components/helpcenter/HelpCenterMenuContent.test.tssrc/components/helpcenter/HelpCenterMenuContent.vuesrc/components/topbar/WorkflowTabs.test.tssrc/components/topbar/WorkflowTabs.vuesrc/extensions/core/cloudFeedbackTopbarButton.test.tssrc/extensions/core/cloudFeedbackTopbarButton.tssrc/platform/support/config.test.tssrc/platform/support/config.ts
📦 Bundle: 5.26 MB gzip 🔴 +334 BDetailsSummary
Category Glance App Entry Points — 22.6 kB (baseline 22.6 kB) • 🔴 +26 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.24 MB (baseline 1.24 MB) • 🟢 -1.52 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 81.8 kB (baseline 81.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 491 kB (baseline 489 kB) • 🔴 +2.01 kBConfiguration panels, inspectors, and settings screens
Status: 22 added / 21 removed User & Accounts — 17.5 kB (baseline 17.5 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 1 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed UI Components — 62.9 kB (baseline 62.9 kB) • 🔴 +22 BReusable component library chunks
Status: 9 added / 9 removed / 5 unchanged Data & Services — 3.04 MB (baseline 3.04 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 365 kB (baseline 365 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 20 added / 20 removed / 11 unchanged Vendor & Third-Party — 9.94 MB (baseline 9.94 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 8.84 MB (baseline 8.84 MB) • 🔴 +29 BBundles that do not match a named category
Status: 116 added / 116 removed / 19 unchanged ⚡ Performance
|
Per CodeRabbit review: prefer getByRole('menuitem', { name: 'Give
Feedback' }) over getByTestId for the help center feedback item to
better validate user-facing accessibility semantics.
|
Thanks for the review. Addressed the nitpick in 6cd1fee — switched the three |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #11863 +/- ##
===========================================
- Coverage 71.48% 56.19% -15.30%
===========================================
Files 1491 1383 -108
Lines 83628 70640 -12988
Branches 22131 19685 -2446
===========================================
- Hits 59784 39695 -20089
- Misses 22983 30429 +7446
+ Partials 861 516 -345
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 992 files with indirect coverage changes 🚀 New features to boost your workflow:
|
dosubot
Bot
added
the
size:M
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Created by the Glary-Bot Agent
Summary
PR #10890 routed the legacy action bar feedback button and the Help Center feedback item to the nightly Typeform survey, but the default topbar feedback button in
WorkflowTabs.vuestill calledbuildFeedbackUrl()and opened Zendesk. SinceComfy.UI.TabBarLayoutdefaults toDefault(notLegacy), most Cloud/Nightly users were clicking the WorkflowTabs button and never reaching the Typeform survey — explaining the lack of survey responses.Changes
Added a shared
buildFeedbackTypeformUrl(source)helper inplatform/support/config.tsthat tags the survey URL with:distribution:ccloud/oss-nightly/oss(preserves the build-tagging the oldbuildFeedbackUrl()sent to Zendesk so responses stay segmented)source:topbar/action-bar/help-center(identifies which UI entry point launched the survey)Tags are passed via the URL fragment (Typeform's hidden-field convention), so they reach the survey but are never sent to the server in the request line.
WorkflowTabs.vue: replacedbuildFeedbackUrl()withbuildFeedbackTypeformUrl('topbar').cloudFeedbackTopbarButton.tsandHelpCenterMenuContent.vue: use the shared builder with their respective source labels instead of inline URL literals.Removed the now-unused
buildFeedbackUrl()andZENDESK_FEEDBACK_FORM_ID(knip-clean).buildSupportUrl()is preserved —Comfy.ContactSupport(the Help Center "Help" item) still routes to Zendesk as before.Added unit tests for the builder, the WorkflowTabs feedback button, the legacy action bar button, and the Help Center feedback item (covering both the Cloud/Nightly Typeform path and the OSS
Comfy.ContactSupportfallback).Verification
pnpm format,pnpm lint,pnpm typecheck,pnpm knip: clean (one pre-existing unrelated lint warning inuseWorkspaceBilling.test.ts)pnpm test:unit(impacted scope): 506/506 passing, including 13 new testsReview Focus
WorkflowTabs.vue(v-if="isCloud || isNightly") is unchanged and matches PR feat: integrate Typeform survey into feedback button #10890's gating philosophy.Comfy.ContactSupportcommand intentionally still route to Zendesk — feedback ≠ support.┆Issue is synchronized with this Notion page by Unito