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: repeat with null #371

Merged
merged 1 commit into from
Apr 12, 2024
Merged

fix: repeat with null #371

merged 1 commit into from
Apr 12, 2024

Conversation

islxyqwe
Copy link
Member

fix the issue that chart cannot render when repeat with null x-axis or y-axis.
image

Copy link

vercel bot commented Apr 12, 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 Apr 12, 2024 11:19am

Copy link
Contributor

github-actions bot commented Apr 12, 2024

Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/lib/vega.ts

The code changes are generally safe and follow good practices. However, there are a few areas that could be improved for readability and maintainability:

  1. Use of any type: The spec variable is declared with any type. This could potentially lead to runtime errors. It's recommended to use a specific type or interface to provide better type safety.
const spec: any = {
  data: {
    values: dataSource,
  },
  params: [
    {
      name: 'geom',
      select: {
        type: 'point',
        fields: geomFieldIds.map(encodeFid),
      },
    },
  ],
};
  1. Complex function: The toVegaSpec function is quite complex and long. It might be beneficial to break it down into smaller, more manageable functions. This would improve readability and maintainability of the code.

  2. Magic numbers: The numbers 5 and 1 are used directly in the code. It's recommended to replace these magic numbers with named constants to improve readability and maintainability.

const PADDING = 5;
const MIN_REPEAT_FIELDS = 1;
  1. Use of let: The let keyword is used for variables that are not reassigned. It's recommended to use const instead to ensure immutability.
const index = 0;
const result = new Array(rowRepeatFields.length * colRepeatFields.length);

🔍📚🔧


Powered by Code Review GPT

@islxyqwe islxyqwe force-pushed the fix-repeat-with-null branch from 6232e1d to cdae200 Compare April 12, 2024 11:14
@islxyqwe islxyqwe merged commit bc39b45 into main Apr 12, 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