Conversation
DuckDbDataSource was used in #contextMenuCopyClicked to check the datasource type but was never imported, causing a ReferenceError whenever the user triggered copy-to-table from the pivot context menu. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug where attempting to copy data from a pivot table to a new table resulted in a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Fixes a runtime ReferenceError in the PivotTable UI context-menu “Copy to table” action by importing the missing DuckDB datasource class used in that code path.
Changes:
- Add the missing
DuckDbDataSourceimport toPivotTableUi.jssoDuckDbDataSource.parseId()andDuckDbDataSource.types.FILESare defined at runtime.
You can also share your feedback on Copilot code review. Take the survey.
ExportUi.exportDataForQueryModel() is called in #contextMenuCopyClicked (line 2607) for the copy-to-table action but was also never imported, meaning the feature would have thrown a ReferenceError immediately after the DuckDbDataSource error was resolved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a ReferenceError for DuckDbDataSource by adding the necessary import statement in PivotTableUi.js. My review includes a suggestion to also import ExportUi, which appears to be used without being imported in the same file, likely causing another ReferenceError.
| import { FilterDialog } from '../FilterUi/FilterUi.js'; | ||
| import { PivotTableUiHighlighting } from './PivotTableUiHighlighting.js'; | ||
| import { showErrorDialog } from '../ErrorDialog/ErrorDialog.js'; | ||
| import { DuckDbDataSource } from '../DataSource/duckdb/DuckDbDataSource.js'; |
There was a problem hiding this comment.
While this change correctly fixes the ReferenceError for DuckDbDataSource, I've noticed that ExportUi is also used in the #contextMenuCopyClicked method (line 2607) without being imported. This will likely cause a similar ReferenceError.
To prevent this, I've added the import for ExportUi. I've placed it before the DuckDbDataSource import to group it with other UI-related imports.
| import { DuckDbDataSource } from '../DataSource/duckdb/DuckDbDataSource.js'; | |
| import { ExportUi } from '../ExportUi/ExportUi.js'; | |
| import { DuckDbDataSource } from '../DataSource/duckdb/DuckDbDataSource.js'; |
sqlOptions was declared with const at line 245 but referenced at lines 237 and 242, causing a ReferenceError (Cannot access 'sqlOptions' before initialization) whenever copy-to-table was triggered from the pivot context menu. Move the declaration before the if/else block that uses it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e path - Add missing imports: QueryAxisItem (from QueryModel.js), SqlQueryGenerator, getComma, quoteIdentifierWhenRequired to ExportDialog.js - Pass exportSettings.exportType to getExportSqlForQueryModel in exportDataForQueryModel - Move busyDialog.close() into the finally block in PivotTableUi.js - Add SqlQueryGenerator mock to lifecycle test to prevent transitive import issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…u actions
Wrap each case body in braces {} so that let declarations (cell, columnIndex,
row, rowIndex) are scoped to their own block and don't cause TDZ errors when
other cases execute first.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After wrapping the CopyRow case in {}, the previously-leaked `let cell` from
CopyColumn is no longer in scope. Add an explicit `let cell` declaration inside
the CopyRow block before the loop that uses it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ases columnsAxisItems can be undefined when no column axis items exist. Add null guard before .length access to prevent TypeError at runtime. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two issues combined to cause this: 1. JS used body.clientWidth/Height for viewport bounds, which can be 0 when the app layout doesn't expand the body. Switch to window.innerWidth/ innerHeight and use clientX/clientY (viewport coords) instead of pageX/pageY to match position:fixed coordinate space. 2. CSS used position:absolute without resetting the UA popover default inset:0; margin:auto, causing clamping to (0,0). Switch to position:fixed and add inset:auto to neutralise the UA defaults. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
With position:fixed;inset:auto, the element's default position before any inline style is applied is (0,0). Pre-set style.left/top before calling showPopover() so the element is never painted at the wrong position. Also adds viewport-edge correction for submenus (flip to left side of trigger when submenu would overflow the right or bottom edge). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bug
Right-clicking a pivot table result and selecting Copy to table from the context menu threw:
Root cause
PivotTableUi.jsusesDuckDbDataSource.parseId()andDuckDbDataSource.types.FILESin#contextMenuCopyClicked(lines 2600–2601) but the class was never imported.Fix
Add the missing import:
Test plan
🤖 Generated with Claude Code