Adds ability to select graph detail (interval) #1574
Conversation
Next steps: - CH stuff for stat selection - Interval selection UI - Interval selection CH - Tests
…s, ToP from graphing
…rounded) top stats
vinibrsl
force-pushed
the
graph-interval-2
branch
from
af5a6e9
to
5cc5e60
Compare
| @@ -387,72 +387,6 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do | |||
| describe "GET /api/stats/main-graph - varying intervals" do | |||
| setup [:create_user, :log_in, :create_new_site] | |||
|
|
|||
| test "displays visitors for a month on an hourly scale", %{conn: conn, site: site} do | |||
There was a problem hiding this comment.
I removed these tests because as of 6076650384c1240f958784616686e91b5938f47c, these period + interval combinations are not supported, and we already have tests for invalid combinations.
|
I think I got every code review comment covered. Thanks all for your inputs 🙌 I pushed one extra commit reviewing the overall user experience, including smooth transitions, dark mode support, fixed a bug where the interval input would blink when changing intervals, etc. I also implemented @ukutaht's suggestion of displaying the selected interval replacing the "Graph detail" label.
I tested the CSV export, and it is not taking intervals into account. We should definitely fix that. Can we do that in a separate pull request, as I'm guessing that would be mainly back-end code? I feel like this pull request is starting to grow again, and I want to avoid that. If you have some time, please give this branch a try. Seeding the database may help. Let me know if there are any other UX improvements I can make, or bugs you find. |
i don't know how to give this branch a try but i'd love to test this feature and send any feedback in order not to delay things too much. for this purpose, it would be nice if we can get these types of features that make important visual changes to the UI in staging itself so everyone can test there with for instance our live demo data. |
We don't have live demo data mirroring from prod->staging yet. Staging only has some fake data. Not great for UI testing because the dashboard doesn't 'feel real'. We can implement the live mirroring at some point next year for some public dashboards that are good for testing. Correct me if I'm wrong @vinibrsl but the best way to test this right now is in production. It will be deployed behind a feature flag which means it can be enabled for your personal site @metmarkosaric for testing before we enable it for the public. We can use the same approach for comparisons, funnels and other new features that don't modify existing UI too much. |
You're right! The best path for assessing this feature is in production, as we don't have real data patterns in staging yet. It is under a feature flag, so we should be good to go. Once it's deployed I'll add your user IDs to the feature flag. |
|
nice one, thank you! |
|
Amazing work, @Vigasaurus! We'll release this feature incrementally during the week 💙 |
|
Awesome - thanks! For the record - I did some quick testing (including with production demo data via API route rewrites) and it seems to function super well from what I can see! Really was quite interesting to see y'all's more robust review and iteration cycle on PRs now, honestly appreciated a lot of the comments on my (albeit somewhat old) code throughout. Excited to see this feature roll out! |
Changes
Recreation of #1366 rebased on top of the newly updated #1364 with several improvements & fixes (again, this should only be merged after #1364)
I could potentially see some benefit here in keeping the interval prop in the query schema to allow for sharing a link with the interval pre-selected, but I figured the better UX of not reloading the entire page on updating the interval was a better tradeoff.
Tests
Changelog
Documentation
Dark mode
Screenshots