Add an opt-in polar grid to the 1.x branch

[wwwaker]

What this changes

• Adds polar grid rendering and configurable radial/angular ticks.
• Keeps grid lines and plotted curves within the same plot clipping area.
• Makes angular labels opt-in (polar.angularLabels: true), using radians by default.

Open design question

When angular labels are enabled, the current implementation selects a
visible radial ring independently for each angle. After panning or zooming,
labels may therefore appear on different rings.

I think we should select one shared ring, favor an outer ring with enough
visible space for labels, and hide labels that do not fit (I learn it from Desmos). I would appreciate
your thoughts on that rule.

Validation

• TypeScript compilation and 14 polar unit tests pass.
• Browser E2E tests have been added.

[wwwaker]

Actually, I’ve considered three approaches to positioning angular tick labels:

1. Place each label where its ray meets the viewport boundary (the approach used in my previous commit).
2. For each ray, place its label where it intersects that ray’s outermost visible grid circle (the current implementation).
3. Select one suitable grid circle for the entire viewport, and show labels only where that circle intersects a ray within the viewport (similar to Desmos’s approach).

[mauriciopoppe]

Please change the base branch to 1.x, the reason is that this is a feature so I'd need to create the branch release-1.26.

[mauriciopoppe]

All the examples are in site/js/site.js, please move this example alongside a comment above it, the dev server running in localhost:8080 should display it.

[mauriciopoppe]

It feels a little weird to have two locations where xDomain & yDomain are set (the block above this one and this one).

I'm thinking we could create a function that returns the xDomain and yDomain, and within that function call getPolarDomains if needed or fallback to the other cartesian domain logic.

function getDomains(
  options: FunctionPlotOptions,
  width: number,
  height: number
): {
  xDomain: number[]
  yDomain: number[]
} {
  const computeYDomainSize = (xDomain: number[]) => {
    const xSize = xDomain[1] - xDomain[0]
    return (height * xSize) / width
  }

  const radius =
    options.coordinateSystem === 'polar'
      ? options.polar?.radiusDomain?.[1]
      : undefined

  const radiusLimit = radius === undefined ? undefined : radius * 1.2

  const xDomain = (() => {
    if (options.xAxis.domain) {
      return options.xAxis.domain
    }

    if (options.xAxis.type === 'log') {
      return [1, 10]
    }

    if (options.xAxis.type === 'linear') {
      return radiusLimit === undefined
        ? [-6, 6]
        : [-radiusLimit, radiusLimit]
    }

    throw new Error(`axis type ${options.xAxis.type} unsupported`)
  })()

  const yDomain = (() => {
    if (options.yAxis.domain) {
      return options.yAxis.domain
    }

    if (options.yAxis.type === 'log') {
      return [1, 10]
    }

    if (options.yAxis.type === 'linear') {
      if (radiusLimit !== undefined) {
        return [-radiusLimit, radiusLimit]
      }

      const ySize = computeYDomainSize(xDomain)
      return [-ySize / 2, ySize / 2]
    }

    throw new Error(`axis type ${options.yAxis.type} unsupported`)
  })()

  if (options.coordinateSystem === 'polar') {
    return getPolarDomains(xDomain, yDomain, width, height)
  }

  return { xDomain, yDomain }
}

Caller:

const { xDomain, yDomain } = getDomains(
  this.options,
  this.meta.width,
  this.meta.height
)

this.meta.xDomain = xDomain
this.meta.yDomain = yDomain

[mauriciopoppe]

self.isPolarCoordinateSystem() instead (where self = outer scope).

[mauriciopoppe]

self.isPolarCoordinateSystem() instead (where self = outer scope).

[mauriciopoppe]

Thanks for catching this!

[mauriciopoppe]

Some of these variables are used within functions e.g. radius is only used by getRayGeometry, please limit the scope of a variable to the smallest scope possible, review this for all the consts you defined here.

[mauriciopoppe]

Please add a docstring in all the functions, if possible add some explanations with ascii graphs about the math.

[mauriciopoppe]

API wise looks clean, I explored the following but I think a similar version it would be part of the v2 (main) branch instead.

interface AxisOptions {
  domain?: [number, number]
  label?: string
}

interface CartesianAxisOptions extends AxisOptions {
  type?: 'linear' | 'log'
  invert?: boolean
  position?: 'sticky' | 'left' | 'bottom'
}

interface PolarRadiusAxisOptions extends AxisOptions {
  ticks?: number | number[]
  tickFormat?: (radius: number) => string
}

interface PolarAngleAxisOptions extends AxisOptions {
  ticks?: number | number[]
  unit?: 'radians' | 'degrees'
  tickFormat?: (angle: number) => string
  labels?: boolean
}

interface PolarOptions {
  grid?: boolean
  radiusAxis?: PolarRadiusAxisOptions
  angleAxis?: PolarAngleAxisOptions
}

[mauriciopoppe]

Please add a test to test/e2e/graphs.test.js too to see a screenshot

[mauriciopoppe]

These are good e2e tests, in this codebase I just did screenshot tests and that was enough but checking dom properties seems like a good idea.

[wwwaker]

Thank you very much for the detailed review and helpful suggestions.
I pushed a follow-up commit addressing the review feedback. The changes include the domain calculation refactor, polar canvas dimension tests, join/enter/exit comments, and the screenshot test.