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

Issue 435 display sensor charts on asset page #449

Merged
merged 96 commits into from
Jul 12, 2022

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Jun 23, 2022

This PR puts sensor data to the forefront of the asset page. The form for editing the asset (which used to be front and center) has been moved to a new sidepanel.

Peek 2022-06-23 17-35

Right now, 2 sensors are shown by default (as seen above), but it is possible to customize which sensors are shown by defining a generic asset attribute (which isn't supported yet by API or UI at this point):

{
    "sensors_to_show": [3, 11, 4, 5]
}

In this example, sensors 3, 11, 4 and 5 would be shown from top to bottom, in the given order.

Flix6x added 30 commits June 23, 2022 16:19
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…at the bottom of the calendar

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…d in the y-axis label

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
… being on the left

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…vigation can still be used

Signed-off-by: F.N. Claessen <felix@seita.nl>
…st the sidepanel background

Signed-off-by: F.N. Claessen <felix@seita.nl>
… the navbar-header accordingly

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…a dictionary with vega-lite specs

Signed-off-by: F.N. Claessen <felix@seita.nl>
…st of years with the pointer. This stops the side panel from collapsing and reopening.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Makes a very clean and visually stable impression now!

One small problem: On the asset page, when viewing a plot with partial data, if I move the cursor into non-data territory, the last data tooltip shown will keep being visible. The sensor page does not seem to have this.

I recall it was possible in other projects we did to zoom in by selecting a range directly on the Altair plots. Is the plan that we'll get that possibility here, as well?

Base automatically changed from styling-user-pages to main July 8, 2022 22:46
@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 8, 2022

One small problem: On the asset page, when viewing a plot with partial data, if I move the cursor into non-data territory, the last data tooltip shown will keep being visible. The sensor page does not seem to have this.

Can you elaborate on the problem? Do you think that the last data tooltip should not be visible in that case, or rather that the sensor page should get the same tooltip behaviour? FYI, underlying the datapoint selection for the tooltip is a hidden voronoi that extends to the sides of the chart area.

@nhoening
Copy link
Contributor

nhoening commented Jul 8, 2022

If I'm not hovering over data, I should not see a data tooltip anymore. So I feel the sensor page has it correct.

@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 8, 2022

I recall it was possible in other projects we did to zoom in by selecting a range directly on the Altair plots. Is the plan that we'll get that possibility here, as well?

I haven't planned it as a FlexMeasures feature right now.

@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 9, 2022

If I'm not hovering over data, I should not see a data tooltip anymore. So I feel the sensor page has it correct.

I disagree. The ability to select the nearest data point this way was advised to me by a UX designer. I agreed with that advice, because it can be quite cumbersome to select individual data points. The sensor page currently shows bar graphs, which makes that easier, except for bars showing values near zero.

A weaker version of your statement I would consider, though: "If I'm not hovering over a period with data, I should not see a data tooltip anymore."

The ability to limit the voronoi to something other than the edges of the chart does not exist right now, however. I think it would require a front-end developer to make that work.

@nhoening
Copy link
Contributor

I can't manage to record my screen properly right now, but I believe we might actually already agree.

In my example, I select a month (I wasn't sure which days actually had data), and so my result has only a few days with data, the rest of the days are blank. If my pointer moves sideways from days with data to days without data, the tooltip stays. Even if I'm two weeks earlier in the blank zone, even if I then leave the plot to the top and come back.

A weaker version of your statement I would consider, though: "If I'm not hovering over a period with data, I should not see a data tooltip anymore."

I think this matches my grief.

@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 10, 2022

Alright, then I understand the grief now. Personally I like the current behaviour, so I can quickly get to see from when the last data point was. But in any case, I can't change it.

Another issue I encountered is that the date selection resets to the most recent available data when you load an asset or sensor page. I was thinking of adding the date selection to the session, so that the same period will load when a user switches to a different page. Would that be better than the current behaviour?

Anything else for this PR?

@nhoening
Copy link
Contributor

nhoening commented Jul 10, 2022

I understand you get some value out of this UX as a power user, but first-time users will be quite confused, I feel.

I can't change it.

I thought the weaker version of my statement is in the realms of possible fixable?

As for the date range selection in the session, I like a selected range to persist across these data plot pages. We have start_time and end_time in the session variables (see ui/utils/view_utils.py::render_flexmeasures_template and set_time_range_for_session) - maybe that is where this should be going?
If that is a lot of work, we can also make a new issue for that.

…sor_charts_on_asset_page

# Conflicts:
#	documentation/changelog.rst
#	flexmeasures/data/models/charts/belief_charts.py
#	flexmeasures/data/models/charts/defaults.py
#	flexmeasures/data/models/time_series.py
#	flexmeasures/ui/static/css/flexmeasures.css
#	flexmeasures/ui/templates/views/sensors.html
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…m some time interval to a sub-interval.

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 11, 2022

I thought the weaker version of my statement is in the realms of possible fixable?

I managed to create another invisible layer myself, as I imagined it might be possible to create data-encoded bars that extend from bottom to top, and assign the tooltip to that.

As for the date range selection in the session, I like a selected range to persist across these data plot pages. We have start_time and end_time in the session variables (see ui/utils/view_utils.py::render_flexmeasures_template and set_time_range_for_session) - maybe that is where this should be going? If that is a lot of work, we can also make a new issue for that.

I looked into it, and it seems like work for a separate PR. We don't want it to be necessary to reload the page in order to set a session variable, so I was thinking we could have relevant API endpoints (like /chart or /chart_data), or a new dedicated API endpoint (called when selecting a calendar period), to set the session variable, instead of doing that when the server is called to load the page.

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x requested a review from nhoening July 11, 2022 15:19
@nhoening
Copy link
Contributor

I think you are right about the session date range deserving its own PR (make a new issue for now?).

I believe the /chart_data endpoint is a good place.

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Thanks for the effort with the tooltip!

…sor_charts_on_asset_page

# Conflicts:
#	flexmeasures/api/dev/sensors.py
#	flexmeasures/data/models/time_series.py
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x merged commit 55589f3 into main Jul 12, 2022
@Flix6x Flix6x deleted the Issue-435_Display_sensor_charts_on_asset_page branch July 12, 2022 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display sensor charts on asset page
3 participants