Added the sticky option to center Axes

[simone-panico]

Added the option position: sticky to the axis, when set the axis will float in the viewport if the origin is visible.

Example:

      functionPlot({
        target: '#playground',
        title: 'quadratic with different axes position',
        width: 580,
        height: 400,
        xAxis: {
          position: 'sticky'
        },
        yAxis: {
          position: 'left'
        },
        data: [{
          fn: 'x^2'
        }]
      })

Ref #291

[mauriciopoppe]

Thank you for your contribution, for the first review I left a comment about the API design.

A few other notes:

• Please add a snapshot test in https://github.com/mauriciopoppe/function-plot/blob/master/test/e2e/snippets.ts
• Please run the code formatter tool https://prettier.io/docs/en/cli to keep the same code style.

[mauriciopoppe]

Comment about API design, what about making this an option of the xAxis, yAxis fields like position: 'sticky' e.g.

    xAxis: {
      label: 'x - axis',
      position: 'sticky',
    },
    yAxis: {
      label: 'y - axis'
      position: 'sticky',
    },

• Why sticky? Seems like a value that matches what the position field in CSS https://developer.mozilla.org/en-US/docs/Web/CSS/position
• What should be the default value for position? Given that this is a new option it should have a default that's backward compatible, possible values could be ['bottom', 'left', 'sticky'] with the defaults:

    xAxis: {
      label: 'x - axis',
      position: 'bottom',
    },
    yAxis: {
      label: 'y - axis'
      position: 'left',
    },

[mauriciopoppe]

This file is autogenerated, first please address the API design comment and we can come back to this file later.

[mauriciopoppe]

Thanks a lot for adding a test and using prettier to format the code! I pulled the changes locally and it looks great, I dragged a graph and I see the axis stick correctly to all the locations and floating when either x=0 or y=0 which is good.

I left more comments, also I noticed that 0 is displayed in both axis, it's a tiny nit but address it if you can.

[mauriciopoppe]

I see this line in two places:

        .select('.x.axis')
        .attr('transform', 'translate(0,' + yTranslation + ')')

...

        .selectAll('.x.axis path, .x.axis line')
        .attr('transform', 'translate(0,' + yTranslation + ')')

Wouldn't the first one automatically apply a translation to the entire group (including the path and line)?

[simone-panico]

No, those are different values. I don't know why, but when I draw the x lines only with yTranslation, it's buggy, and the grid doesn't render correctly.

.attr('transform', 'translate(0,' + (this.meta.height / 2 - yTranslation + this.meta.height / 2) + ')');

I use this for all Paths and Lines

[mauriciopoppe]

There are some attr that are reapplied later e.g. line 700:

    canvas
      .selectAll('.axis path, .axis line')
      .attr('fill', 'none')
      .attr('stroke', 'black')
      .attr('shape-rendering', 'crispedges')
      .attr('opacity', 0.1)

Maybe the lines in 700 should be kept and then remove these additional attrs.

[mauriciopoppe]

I believe this could be introduced as a single commit, please squash all of your commits into one, for more info about squashing commits please read https://kubernetes.io/docs/contribute/new-content/open-a-pr/#squashing-commits.

If it's a single commit then it could easily be cherrypicked into the release-1.24 branch or some other commit, the master branch has lots of changes for v2.0 so it'll be harder to create 1.25 out of it but I can create it from the base of release-1.24

[simone-panico]

Now all commits are squashed in one. Can you tell me if I did it right? I did this the first time.

[mauriciopoppe]

Last comments, they're mostly nits about how where the variables should exist.

Please also update the title of the PR and the description to match what you did.

Thanks a lot for contributing! I'll add an example in the main webpage about using this option.

[mauriciopoppe]

Nit: All of these variables are only used inside their corresponding ifs, consider moving them to be inside them so that they're properly scoped.

[mauriciopoppe]

Nit: Please fix the indentation.

[mauriciopoppe]

I think you might want to amend the commit so that it describes what the commit is about i.e. Add the sticky positioning to center the origin axes if they're in the viewport.