Add an opt-in polar grid to the 1.x branch by wwwaker · Pull Request #373 · mauriciopoppe/function-plot

wwwaker · 2026-09-29T12:16:00Z

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.

Fixes #371

wwwaker · 2026-09-29T13:26:53Z

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

Place each label where its ray meets the viewport boundary (the approach used in my previous commit).
For each ray, place its label where it intersects that ray’s outermost visible grid circle (the current implementation).
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 · 2026-09-30T02:24:17Z


 [`Check the available options in the docs`](https://mauriciopoppe.github.io/function-plot/docs/functions/default-1.html)

+Polar functions use Cartesian coordinates by default. To display a polar grid,


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 · 2026-09-30T02:27:19Z

      throw Error('axis type ' + axis.type + ' unsupported')
    })(this.options.yAxis))

+    if (this.options.coordinateSystem === 'polar') {


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 · 2026-09-30T02:30:19Z

      .selectAll('text.x.axis-label')
      .data(function (d: FunctionPlotOptions) {
-        return [d.xAxis.label].filter(Boolean)
+        return d.coordinateSystem === 'polar' ? [] : [d.xAxis.label].filter(Boolean)


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

mauriciopoppe · 2026-09-30T02:30:51Z

      .selectAll('text.y.axis-label')
      .data(function (d: FunctionPlotOptions) {
-        return [d.yAxis.label].filter(Boolean)
+        return d.coordinateSystem === 'polar' ? [] : [d.yAxis.label].filter(Boolean)


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

mauriciopoppe · 2026-09-30T02:31:05Z

        .attr('stroke', 'black')
        .attr('opacity', 0.2)
      yOrigin.merge(yOriginEnter).attr('d', this.line)
+      yOrigin.exit().remove()


Thanks for catching this!

mauriciopoppe · 2026-09-30T03:17:24Z

+  const polar = options.polar || {}
+  const xScale = meta.xScale
+  const yScale = meta.yScale
+  const visibleDomain = getPolarRadiusDomain(xScale.domain(), yScale.domain())


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 · 2026-09-30T03:19:16Z

+const TWO_PI = 2 * Math.PI
+const DEFAULT_POLAR_ANGLE_UNIT = 'radians'
+
+export function validatePolarOptions(options: FunctionPlotOptions) {


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

mauriciopoppe · 2026-09-30T03:20:35Z

  position?: 'sticky' | 'left' | 'bottom'
 }

+export interface PolarOptions {


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 · 2026-09-30T03:22:00Z

@@ -0,0 +1,187 @@
+const puppeteer = require('puppeteer')


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

mauriciopoppe · 2026-09-30T03:23:54Z

+    await browser?.close()
+  })
+
+  it('renders a true polar grid while preserving Cartesian defaults', async () => {


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 · 2026-09-30T07:17:41Z

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.

mauriciopoppe · 2026-10-03T16:20:16Z

test failed because the chain of commits should also include the snapshotted image, please generate one.

FAIL test/e2e/graphs.test.js (20.707 s)
  ● Function Plot › should render a polar grid
    New snapshot was not written. The update flag must be explicitly passed to write a new snapshot.
     + This is likely because this test is run in a continuous integration (CI) environment in which snapshots are not written by default.
      104 |     `)
      105 |     const image = await page.screenshot()
    > 106 |     expect(image).toMatchImageSnapshot(matchSnapshotConfig)
          |                   ^
      107 |   })
      108 |
      109 |   it('should render distinct domains', async () => {
      at toMatchImageSnapshot (test/e2e/graphs.test.js:106:19)
      at call (test/e2e/graphs.test.js:2:1)
      at Generator.tryCatch (test/e2e/graphs.test.js:2:1)
      at Generator._invoke [as next] (test/e2e/graphs.test.js:2:1)
      at asyncGeneratorStep (test/e2e/graphs.test.js:2:1)
      at asyncGeneratorStep (test/e2e/graphs.test.js:2:1)
A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.

wwwaker · 2026-10-08T12:52:41Z

The missing snapshot baseline is now included. I've also improved the polar grid labels and clipping, and fixed zoom continuity across resizing and coordinate-system switches, with regression tests.
Local unit tests, browser tests, and the polar snapshot comparison pass. Could you take another look?

mauriciopoppe · 2026-10-09T00:58:52Z

-    // update the range only but not the domain, the domain is going to be updated
-    self.meta.zoomBehavior.xScale.range(self.meta.xScale.range())
-    self.meta.zoomBehavior.yScale.range(self.meta.yScale.range())
+    if (this.isPolarCoordinateSystem() || this.getEmitInstance().wasPolarCoordinateSystem) {


In the v2 version I'm thinking about removing the update hook which means that I might not carry over this change, the reason is that I dislike the current way of updating things e.g.

// initial render const opts = {} functionPlot(opts) // update requires setting properties on the object sent. opts.foo = bar functionPlot(opts)

I imagine that this update might be for that type of update from cartesian to polar?
In any case handling the update seems good to me.

mauriciopoppe · 2026-10-09T01:02:20Z

   */
  private linkedGraphs: Array<Chart>
  private line: Line<[number, number]>
+  // Keep the rendered mode separately because callers can mutate and reuse options.


A chart holds the metadata for the axis under the options, maybe this property should be a hidden property there and not in the chart. Can this be derived instead?

wwwaker · 2026-10-09T02:25:05Z

Thanks for the suggestion. I'll move the last rendered coordinate-system state into an internal property on options.

My concern with deriving the previous mode is that the existing state doesn't provide a reliable source: options.coordinateSystem may already be changed, and the layout and scales have been updated by the time the zoom baseline is rebuilt. Grid presence isn't reliable either, since polar.grid can be disabled.
Keeping the last rendered mode as internal metadata on options makes the transition check explicit and independent of those layout and visibility details.

wwwaker · 2026-10-09T03:12:14Z

To clarify the cause with a concrete example: for a 550 × 550 chart without a title, polar mode uses the full 550 × 550 plot area because its margins are zero. Cartesian mode reserves 40px on the left, 20px on the right, and 20px on both the top and bottom, leaving a 490 × 510 plot area.
Previously, switching modes only updated the zoom baseline’s pixel range. It was no longer properly aligned with the rebuilt viewport and existing transform, so the next wheel event caused a jump.
Tracking the last rendered mode lets us detect this transition and recalibrate the zoom baseline while preserving the current zoom and pan state.

mauriciopoppe · 2026-10-09T17:19:39Z

Thank you for your contribution @wwwaker! I'll publish a 1.26 release soon.

wwwaker added 2 commits September 29, 2026 09:38

feat: add opt-in polar coordinate grid

4a2633c

fix: refine polar grid labels and clipping

dab5722

mauriciopoppe reviewed Sep 30, 2026

View reviewed changes

wwwaker marked this pull request as draft September 30, 2026 05:03

wwwaker changed the base branch from release-1.25 to 1.x September 30, 2026 05:05

refactor: address polar grid review feedback

924f094

wwwaker marked this pull request as ready for review September 30, 2026 07:20

test: add polar grid image snapshot

6564d20

wwwaker marked this pull request as draft October 8, 2026 10:36

fix: improve polar grid layout and preserve zoom

a0619c7

wwwaker force-pushed the codex/polar-grid-release-1.25 branch from 0fb9e4e to a0619c7 Compare October 8, 2026 12:44

wwwaker marked this pull request as ready for review October 8, 2026 12:47

mauriciopoppe approved these changes Oct 9, 2026

View reviewed changes

refactor: store coordinate state in options

922a029

mauriciopoppe merged commit 287e5f2 into mauriciopoppe:1.x Oct 9, 2026
3 of 4 checks passed


		[`Check the available options in the docs`](https://mauriciopoppe.github.io/function-plot/docs/functions/default-1.html)

		Polar functions use Cartesian coordinates by default. To display a polar grid,

Conversation

wwwaker commented Sep 29, 2026 • edited by mauriciopoppe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this changes

Open design question

Validation

Uh oh!

wwwaker commented Sep 29, 2026

Uh oh!

mauriciopoppe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwwaker commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mauriciopoppe commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wwwaker commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwwaker commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wwwaker commented Oct 9, 2026

Uh oh!

Uh oh!

mauriciopoppe commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wwwaker commented Sep 29, 2026 •

edited by mauriciopoppe

Loading

wwwaker commented Sep 30, 2026 •

edited

Loading

mauriciopoppe commented Oct 3, 2026 •

edited

Loading

wwwaker commented Oct 9, 2026 •

edited

Loading