-
Notifications
You must be signed in to change notification settings - Fork 16
Add optional blankConnectionName prop to BlankViewerProps #412
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
…ection name Co-authored-by: ben-polinsky <78756012+ben-polinsky@users.noreply.github.com>
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 adds an optional blankConnectionName prop to BlankViewerProps, allowing users to customize the connection name for blank iModel connections instead of being limited to the hardcoded "Blank Connection" string.
Changes:
- Added optional
blankConnectionNameprop to theBlankViewerPropstype definition - Updated connection opening logic to use the custom name when provided, with a fallback to the default "Blank Connection"
- Added test coverage for both custom and default connection name scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/modules/viewer-react/src/types.ts | Adds optional blankConnectionName property to BlankViewerProps interface |
| packages/modules/viewer-react/src/services/iModel/iModelViewerHelper.ts | Updates openConnection() and gatherRequiredViewerProps() to handle the new optional prop |
| packages/modules/viewer-react/src/tests/services/iModelViewerHelper.test.ts | Adds test cases verifying custom and default blank connection name behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ben-polinsky
left a comment
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.
You need to run rush change
Co-authored-by: ben-polinsky <78756012+ben-polinsky@users.noreply.github.com>
Done in commit 04e5ca6 |
ben-polinsky
left a comment
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.
build's failing
The build failure is due to a pre-existing warning about outdated browserslist data (caniuse-lite) in |
@copilot Can you, like, do that? |
Implementation Complete ✅
blankConnectionNameprop toBlankViewerPropstype intypes.tsopenConnectionfunction iniModelViewerHelper.tsto use the optional prop instead of hardcoded stringgatherRequiredViewerPropsto pass through theblankConnectionNamepropSummary
Successfully implemented the ability to customize the blank connection name through an optional
blankConnectionNameprop. The implementation is fully backward compatible - when the prop is not provided, it defaults to "Blank Connection" (the existing behavior).Changes
blankConnectionName?: stringtoBlankViewerPropsopenConnectionto use the optional prop with a default fallback, andgatherRequiredViewerPropsto pass through the propAll tests pass (43/43) and the code is clean with no security vulnerabilities.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.