Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-9343

scatter and line chart zoom changed when changing between "points" and "connected points"

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Firefly
    • Labels:
    • Story Points:
      6
    • Sprint:
      SUIT Sprint 2017-2, SUIT Sprint 2017-3
    • Team:
      Science User Interface

      Description

      Clicking on the gear in the chart display, zoom in using rubber band, then change to another option "points", "connected points", the chart was reset back before the zoom.

      We should keep the chart zoom unchanged.

      Proposed behavior:
      If the data to be plotted changes, yes, the zoom level should be reset.
      But if the data does not change and only the style of plot changes, the zoom level should remain the same.

      [Feb-27-2017]
      The axis ranges are calculated each time when an action is triggered through the option parameters passed in payload. When Zoom is selected, the zoom (it is a object literal: xMin, xMax, yMin and yMax)
      is added to the options. However, the zoom information is lost when the line style change action is performed. The fix is

      1. Once the Zoom button is clicked, the zoom information is kept until reset zoom is performed
      2. When zoom is reset, the zoom is replaced by the data ranges which stored in the boundaries (xMin, xMax, yMin, yMax).

      To my understanding the boundaries are safe to use because they are only be used when the axis ranges are calculated.

      The alternate way to fix is to change props. But it may lead to make more changes.

      The zoom is not a property in XYOptions. When the zoom is selected, it is added by calling updateSet in the reducer(CHART_OPTIONS_UPDATE). When the line style is change, the zoom information is lost by calling updateMerge in the reducer(CHART_DATA_UPDATE). To keep the zoom, either we add a zoom property in XYOption or not to remove the zoom when calling upateMerge.

      The implementation is as following:

      1. Add a zoom property in XYPlotOptions
      2. Call resultSuccessful in XYPlotOption with xyParameter as an argument that contains zoom, columns etc
      3. At the resultSuccessful, check if x or y column changes, set zoom to undefined, otherwise, set zoom to the xyParameter's zoom value if it exists.

        Attachments

          Issue Links

            Activity

            Hide
            ejoliet Emmanuel Joliet added a comment -

            The zoom is reset every time the use change the options for xy-plot, so it not only a feature for the line chart type, it is implemented for any time xy-plot option changes. This will need more technical discussion on how to solve it.
            Do we want a reset zoom for specific option and keep zoom for others?

            Show
            ejoliet Emmanuel Joliet added a comment - The zoom is reset every time the use change the options for xy-plot, so it not only a feature for the line chart type, it is implemented for any time xy-plot option changes. This will need more technical discussion on how to solve it. Do we want a reset zoom for specific option and keep zoom for others?
            Hide
            xiuqin Xiuqin Wu [X] (Inactive) added a comment -

            My thought:
            If the data to be plotted changes, yes, the zoom level should be reset.
            But if the data does not change and only the style of plot changes, the zoom level should remain the same.

            Show
            xiuqin Xiuqin Wu [X] (Inactive) added a comment - My thought: If the data to be plotted changes, yes, the zoom level should be reset. But if the data does not change and only the style of plot changes, the zoom level should remain the same.
            Show
            zhang Lijun Zhang [X] (Inactive) added a comment - https://github.com/Caltech-IPAC/firefly/pull/321
            Hide
            zhang Lijun Zhang [X] (Inactive) added a comment -

            merged

            Show
            zhang Lijun Zhang [X] (Inactive) added a comment - merged

              People

              Assignee:
              zhang Lijun Zhang [X] (Inactive)
              Reporter:
              xiuqin Xiuqin Wu [X] (Inactive)
              Reviewers:
              Emmanuel Joliet, Tatiana Goldina
              Watchers:
              Emmanuel Joliet, Lijun Zhang [X] (Inactive), Tatiana Goldina, Xiuqin Wu [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.