Skip to content
This repository has been archived by the owner on Jan 23, 2025. It is now read-only.

MG-211 - Update time series bar chart fo fetch data from back-end #224

Merged
merged 9 commits into from
Mar 21, 2024

Conversation

felixgateru
Copy link
Contributor

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Dependency Update

What does this do?

Populate the time series bar chart with data from the back-end

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

  • Yes
  • No, and this is why:

Did you document any new/modified functionality?

  • Yes
  • No, and this is why:

Notes

@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch from 02c677a to 9555d77 Compare March 12, 2024 17:17
@felixgateru felixgateru marked this pull request as ready for review March 13, 2024 09:28
@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch from b1c4bac to fa93291 Compare March 13, 2024 09:30
@dborovcanin dborovcanin force-pushed the MG-211_time_series_bar branch from fa93291 to f367626 Compare March 13, 2024 10:49
Comment on lines 248 to 249
from: ${this.chartData.startTime / 1000},
to: ${this.chartData.stopTime / 1000},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from: ${this.chartData.startTime / 1000},
to: ${this.chartData.stopTime / 1000},
from: ${this.chartData.startTime},
to: ${this.chartData.stopTime},

@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch 3 times, most recently from 90fb2ea to b27f358 Compare March 13, 2024 15:26
throw new Error("HTTP request failed with status: " + response.status);
}
const data = await response.json();
console.log(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove logs

@ianmuchyri
Copy link
Contributor

Also make it such that aggregation is not compulsory. Check how it was changed in the line chart. Aggregation is not compulsory, but if a user selects aggregation, then they have to select interval


async function getData(barChart, chartData) {
try {
const apiEndpoint = "/data?channel=" + chartData.channel +
Copy link
Contributor

Choose a reason for hiding this comment

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

add prefix

}
}
toggleUpdateInterval();
aggregationTypeInput.addEventListener("change", toggleUpdateInterval);
Copy link
Contributor

Choose a reason for hiding this comment

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

 toggleUpdateInterval();
 aggregationTypeInput.addEventListener("change", toggleUpdateInterval);

what is the difference here. Since the function has been called by default in the first instance

chartData[pair[0]] = pair[1];
if (pair[0] === "startTime" || pair[0] === "stopTime") {
const inputDate = new Date(pair[1]);
chartData[pair[0]] = inputDate.getTime() * 1e6;
Copy link
Contributor

Choose a reason for hiding this comment

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

r

Suggested change
chartData[pair[0]] = inputDate.getTime() * 1e6;
chartData[pair[0]] = inputDate.getTime() ;

@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch from 48cf431 to 2ab8736 Compare March 15, 2024 12:01
@@ -226,7 +244,12 @@ <h5 class="modal-title" id="barChartModalLabel">Time Series Bar Chart</h5>
let chartData = {};
let formData = new FormData(form);
for (var pair of formData.entries()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (var pair of formData.entries()) {
for (let [name, value] of formData) {

@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch 2 times, most recently from 11d09c7 to 4ddff5e Compare March 18, 2024 15:53
@@ -216,31 +216,86 @@ class TimeSeriesBarChart extends Echart {
},
xAxis: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
xAxis: {
tooltip: {
trigger: 'axis'
},
xAxis: {

Copy link
Contributor

Choose a reason for hiding this comment

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

for sth like this
image

@felixgateru felixgateru force-pushed the MG-211_time_series_bar branch from 4ddff5e to 2d275cf Compare March 19, 2024 09:52
Copy link
Contributor

@ianmuchyri ianmuchyri left a comment

Choose a reason for hiding this comment

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

LGTM

@dborovcanin dborovcanin force-pushed the MG-211_time_series_bar branch from 2d275cf to 20939d2 Compare March 21, 2024 14:51
@dborovcanin dborovcanin merged commit 2d2b4e5 into absmach:main Mar 21, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants