Skip to content
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

fix: error when pureRenderer displaying pivot table #390

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

islxyqwe
Copy link
Member

@islxyqwe islxyqwe commented Jun 6, 2024

Fixed the error when using purerenderer with a pivot table chart, the component will crash and throw a Cannot read properties of undefined (reading 'timezoneDisplayOffset').

@islxyqwe islxyqwe requested a review from ObservedObserver June 6, 2024 06:50
Copy link

vercel bot commented Jun 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
graphic-walker ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 9:14am

Copy link
Contributor

github-actions bot commented Jun 6, 2024

Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/components/pivotTable/index.tsx

The changes in this file are quite extensive and involve a lot of state management and data manipulation. There are no obvious bugs or performance issues, but the complexity of the code could make it difficult to maintain and debug. Consider adding more comments to explain what each part of the code does. For example:

// This effect triggers when the enableCollapse state or the tableCollapsedHeaderMap state changes. It decides whether to directly generate a new table or to aggregate data before generating the table.
useEffect(() => {...}, [enableCollapse, tableCollapsedHeaderMap]);

Also, consider breaking down complex functions into smaller, more manageable functions. For instance, the aggregateThenGenerate function could be broken down into separate functions for aggregating data and generating the table.


Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/App.tsx

The changes in this file are minimal and seem to be adjusting the minimum and maximum width of a component. This could potentially cause layout issues depending on the rest of the CSS and the expected screen sizes, but without more context it's hard to say for sure. It would be a good idea to test these changes on various screen sizes to ensure there are no layout issues.


📝🔍📏


Powered by Code Review GPT

@islxyqwe islxyqwe marked this pull request as ready for review June 11, 2024 02:50
@islxyqwe islxyqwe changed the title WIP: fix: error when pureRenderer displaying pivot table fix: error when pureRenderer displaying pivot table Jun 19, 2024
@ObservedObserver ObservedObserver merged commit 9c73454 into main Jun 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants