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 animation duration not consistent in TimeSeriesWidget #214

Conversation

Clebal
Copy link
Contributor

@Clebal Clebal commented Oct 26, 2021

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #188731: Animation duration is not consistent in TimeSeriesWidget.

@coveralls
Copy link
Collaborator

coveralls commented Oct 26, 2021

Pull Request Test Coverage Report for Build 1389571138

  • 23 of 45 (51.11%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.2%) to 69.446%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/react-ui/src/widgets/TimeSeriesWidgetUI/TimeSeriesWidgetUI.js 23 45 51.11%
Files with Coverage Reduction New Missed Lines %
packages/react-ui/src/widgets/TimeSeriesWidgetUI/TimeSeriesWidgetUI.js 3 62.8%
Totals Coverage Status
Change from base Build 1386904746: -1.2%
Covered Lines: 888
Relevant Lines: 1195

💛 - Coveralls

currentTimeWindow[0] + msTimeWindowStep,
currentTimeWindow[1] + msTimeWindowStep
];
if (currentTimeWindow[1] > data[data.length - 1].name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Clebal what is data[data.length - 1].name) ? it looks something strange, comparing with .name...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, name is the date resulted from aggregation. I used name following the same prop name that in others widgets. But... I can change it if you want.

@@ -44,7 +44,6 @@ const STEP_SIZE_RANGE_MAPPING = {
* @param {string} props.stepSize - Step applied to group the data. Must be one of those defined in `GroupDateTypes` object.
* @param {string[]} [props.stepSizeOptions] - Different steps that can be applied to group the data. If filled, an icon with a menu appears to change between steps. Every value must be one of those defined in `AggregationTypes` object.
* @param {string} [props.chartType] - Chart used to represent the time serie. Must be one of those defined in `TIME_SERIES_CHART_TYPES` object.
* @param {number} [props.duration] - Duration of the animation in milliseconds. 20s by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A doc update will be needed for this, @borja-munoz

@VictorVelarde VictorVelarde changed the title Animation duration is not consistent in TimeSeriesWidget Fix animation duration not consistent in TimeSeriesWidget Oct 26, 2021
@VictorVelarde VictorVelarde self-requested a review October 27, 2021 09:50
@VictorVelarde VictorVelarde merged commit 2618a47 into master Oct 27, 2021
@VictorVelarde VictorVelarde deleted the bug/sc-188731/animation-duration-is-not-consistent-in-timeserieswidget branch October 27, 2021 09:55
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.

3 participants