-
Notifications
You must be signed in to change notification settings - Fork 4
feat: big number block UX and settings #164
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
Conversation
📝 WalkthroughWalkthroughAdds end-to-end support for configuring "big number" comparison metadata in Deepnote notebooks. Introduces a webview-based settings panel and React bundle, a notebook cell status bar provider with inline commands, new localization keys and types for comparison settings, updates the big-number block converter to use value expressions, and registers the status bar provider in service registries. Persists settings into notebook cell metadata and wires message handling between webview and extension. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant StatusBar as Status Bar
participant Extension as Extension Host
participant Webview as BigNumber Settings Webview
participant Notebook as Notebook Document
User->>StatusBar: invoke "Configure Comparison"
StatusBar->>Extension: command deepnote.configureBigNumberComparison
Extension->>Webview: create panel & post init (loc + settings)
Note over Webview: user edits form
User->>Webview: click Save
Webview->>Extension: postMessage(type: "save", settings)
Extension->>Notebook: apply WorkspaceEdit → update deepnote_big_number_* metadata
Extension->>Webview: respond / dispose panel
Extension->>StatusBar: trigger refresh of status items
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
0da47f1 to
0564a7f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #164 +/- ##
=====================================
Coverage 73% 73%
=====================================
Files 552 552
Lines 44480 44525 +45
Branches 5314 5318 +4
=====================================
+ Hits 32716 32754 +38
- Misses 10001 10004 +3
- Partials 1763 1767 +4
🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
build/esbuild/build.ts(1 hunks)src/messageTypes.ts(1 hunks)src/notebooks/deepnote/bigNumberComparisonSettingsWebview.ts(1 hunks)src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts(1 hunks)src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.ts(5 hunks)src/notebooks/deepnote/deepnoteBigNumberCellStatusBarProvider.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/notebooks/serviceRegistry.web.ts(2 hunks)src/platform/common/utils/localize.ts(1 hunks)src/platform/notebooks/deepnote/types.ts(1 hunks)src/webviews/webview-side/bigNumberComparisonSettings/BigNumberComparisonSettingsPanel.tsx(1 hunks)src/webviews/webview-side/bigNumberComparisonSettings/bigNumberComparisonSettings.css(1 hunks)src/webviews/webview-side/bigNumberComparisonSettings/index.tsx(1 hunks)src/webviews/webview-side/bigNumberComparisonSettings/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/notebooks/deepnote/types.tssrc/platform/common/utils/localize.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.ts
🧬 Code graph analysis (8)
src/webviews/webview-side/bigNumberComparisonSettings/types.ts (1)
src/platform/notebooks/deepnote/types.ts (1)
BigNumberComparisonSettings(30-36)
src/platform/notebooks/deepnote/types.ts (1)
src/webviews/webview-side/bigNumberComparisonSettings/types.ts (1)
BigNumberComparisonSettings(1-7)
src/webviews/webview-side/bigNumberComparisonSettings/BigNumberComparisonSettingsPanel.tsx (4)
src/webviews/webview-side/react-common/postOffice.ts (1)
IVsCodeApi(9-16)src/webviews/webview-side/bigNumberComparisonSettings/types.ts (2)
BigNumberComparisonSettings(1-7)WebviewMessage(9-13)src/notebooks/deepnote/bigNumberComparisonSettingsWebview.ts (1)
handleMessage(184-215)src/webviews/webview-side/react-common/locReactSide.ts (2)
storeLocStrings(19-21)getLocString(11-17)
src/notebooks/deepnote/bigNumberComparisonSettingsWebview.ts (4)
src/notebooks/deepnote/deepnoteBigNumberCellStatusBarProvider.ts (1)
injectable(30-314)src/webviews/webview-side/bigNumberComparisonSettings/types.ts (1)
BigNumberComparisonSettings(1-7)src/platform/notebooks/deepnote/types.ts (2)
BigNumberComparisonSettings(30-36)BigNumberComparisonWebviewMessage(41-45)src/test/mocks/vsc/extHostedTypes.ts (2)
WorkspaceEdit(649-833)NotebookEdit(2467-2530)
src/notebooks/deepnote/deepnoteBigNumberCellStatusBarProvider.ts (3)
src/notebooks/deepnote/bigNumberComparisonSettingsWebview.ts (1)
injectable(28-314)src/platform/deepnote/pocket.ts (1)
src/test/mocks/vsc/extHostedTypes.ts (2)
WorkspaceEdit(649-833)NotebookEdit(2467-2530)
src/webviews/webview-side/bigNumberComparisonSettings/index.tsx (2)
src/webviews/webview-side/react-common/postOffice.ts (1)
IVsCodeApi(9-16)src/webviews/webview-side/bigNumberComparisonSettings/BigNumberComparisonSettingsPanel.tsx (1)
BigNumberComparisonSettingsPanel(11-215)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (3)
src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)src/notebooks/deepnote/deepnoteSchemas.ts (1)
DeepnoteBigNumberMetadataSchema(11-44)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2546-2572)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2546-2572)src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)
🔇 Additional comments (10)
src/platform/common/utils/localize.ts (1)
1159-1176: LGTM!The new localization namespace follows existing patterns and provides all necessary strings for the big number comparison settings UI.
src/notebooks/serviceRegistry.web.ts (2)
53-53: LGTM!Import follows existing pattern.
121-124: LGTM!Registration follows the established pattern for cell status bar providers.
build/esbuild/build.ts (1)
387-392: LGTM!Build step follows the established pattern for webview bundles.
src/webviews/webview-side/bigNumberComparisonSettings/index.tsx (1)
1-20: LGTM!Standard webview entrypoint following established patterns. All necessary setup and rendering code is present.
src/webviews/webview-side/bigNumberComparisonSettings/bigNumberComparisonSettings.css (1)
1-178: LGTM!Clean CSS with proper VS Code theming and accessibility support (focus-visible states).
src/platform/notebooks/deepnote/types.ts (2)
27-36: LGTM!Well-defined interface following existing patterns. The empty string option in
comparisonTypeallows for unset state.
38-45: LGTM!Properly structured discriminated union for webview messaging, following the SelectInputWebviewMessage pattern.
src/notebooks/serviceRegistry.node.ts (2)
75-75: LGTM!Import follows existing pattern.
195-198: LGTM!Registration consistent with the web registry and follows established patterns.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/messageTypes.ts(1 hunks)src/platform/common/utils/localize.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/common/utils/localize.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (1)
src/platform/common/utils/localize.ts (1)
1170-1187: BigNumberComparison namespace looks good.Localization strings are clear and follow established patterns. The help text at line 1181 usefully documents the
{{var}}syntax for variable references.
Summary by CodeRabbit