-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(ingest/ui): Updated lookml recipe and minor ui enhancements #15086
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
base: master
Are you sure you want to change the base?
Conversation
|
✅ Meticulous spotted visual differences in 1 of 1016 screens tested, but all differences have already been approved: view differences detected. Meticulous evaluated ~8 hours of user flows against your PR. Last updated for commit a66ad53. This comment will update as new commits are pushed. |
Bundle ReportChanges will increase total bundle size by 2.15kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
Files in
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…improve Git repository fields
2026f6b to
a66ad53
Compare
| </ul> | ||
| </p> | ||
| </> | ||
| ), |
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.
Looks like this tip is pretty much duplicated, I would use pick one version and share.
| const [searchFilter, setSearchFilter] = useState(''); | ||
|
|
||
| // Callback ref that focuses immediately when the element is attached | ||
| const searchInputCallbackRef = (node: any) => { |
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.
Can we provide a real type instead of any?
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.
(I think it's fine to use any in tests but not in actual code)
| const [searchFilter, setSearchFilter] = useState(''); | ||
|
|
||
| // Callback ref that focuses immediately when the element is attached | ||
| const searchInputCallbackRef = (node: any) => { |
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.
(I think it's fine to use any in tests but not in actual code)
This PR introduces significant improvements to the DataHub ingestion UI, focusing on enhanced user experience and multi-platform Git support.
✨ Key Changes
1. Enhanced SelectTemplateStep Component
2. Multi-Platform Git Repository Support
github_info.repo→git_info.repogithub_info.deploy_key→git_info.deploy_keygit_info.repo_ssh_locatorfield3. Smart SSH Locator Validation
4. Comprehensive Test Coverage (Auto Generated)
🔧 Technical Details
Files Modified:
SelectTemplateStep.tsx: Enhanced with no results message and auto-focuscommon.tsx: Updated Git repository field from GitHub-specific to genericconstants.ts: Updated field references and importslookml.tsx: Complete overhaul of Git-related fields with multi-platform supportFiles Added:
common-git-info.test.tsx: Tests for common Git info fieldsconstants-git-info.test.tsx: Tests for constants integrationgit-info-validation.test.tsx: Tests for validation logiclookml.test.tsx: Comprehensive tests for LookML Git fields🎯 Benefits
🔄 Migration Notes
git_info.*instead ofgithub_info.*📊 Statistics
This enhancement significantly improves the DataHub ingestion experience by supporting multiple Git platforms while maintaining backward compatibility and providing clear user feedback.