-
Notifications
You must be signed in to change notification settings - Fork 17
[Feature] Remind users to connect to data warehouse #846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Remind users to connect to data warehouse #846
Conversation
Signed-off-by: Wei-Chun, Chang <[email protected]>
Signed-off-by: Wei-Chun, Chang <[email protected]>
Signed-off-by: Wei-Chun, Chang <[email protected]>
a14fe63
to
79e2863
Compare
Signed-off-by: Wei-Chun, Chang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a feature to remind users to connect to their data warehouse when running in metadata-only mode. The changes primarily focus on adding UI components and popover guidance to encourage users to set up data warehouse connections when certain query functions are disabled.
- Adds a new
SetupConnectionPopover
component that displays when hovering over disabled query functions - Introduces a
SetupConnectionGuide
component to replace the base environment setup guide in metadata-only mode - Updates various UI components to show connection setup prompts when appropriate
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
js/src/constants/urls.ts | Adds calendar URL constant for support |
js/src/components/query/SqlEditor.tsx | Renames prop from BaseEnvironmentSetupGuide to SetupGuide |
js/src/components/query/SetupConnectionGuide.tsx | New component for data warehouse connection setup guide |
js/src/components/query/QueryPage.tsx | Integrates setup connection guidance into query page |
js/src/components/lineage/SeverDisconnectedModalContent.tsx | Updates support URL reference |
js/src/components/lineage/SetupConnectionBanner.tsx | New banner component for connection setup |
js/src/components/lineage/NodeView.tsx | Wraps menu items with connection setup popover |
js/src/components/lineage/NodeTag.tsx | Adds setup popover to row count diff tag |
js/src/components/lineage/LineageViewTopBar.tsx | Wraps menu items with setup popover |
js/src/components/lineage/LineageViewContextMenu.tsx | Adds setup popover to disabled context menu items |
js/src/components/lineage/LineageView.tsx | Integrates setup banner and removes server flag dependency |
js/src/components/check/CheckDetail.tsx | Adds setup popover to run query button |
js/src/components/app/SetupConnectionPopover.tsx | New popover component for setup guidance |
js/src/components/app/AvatarDropdown.tsx | Updates support URL reference |
Comments suppressed due to low confidence (1)
js/src/components/lineage/LineageViewContextMenu.tsx:69
- [nitpick] The variable name 'menuItem' is generic and could be more descriptive. Consider renaming to 'menuItemElement' or 'contextMenuItem' to better indicate what it represents.
const menuItem = (
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes to grammar
Signed-off-by: Wei-Chun, Chang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR checklist
What type of PR is this?
Feature
What this PR does / why we need it:
Add connecting to warehouse reminder on UI
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: