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

Feat: Time comparison for dimensions charts #5419

Merged
merged 23 commits into from
Nov 1, 2024

Conversation

djbarnwal
Copy link
Member

@djbarnwal djbarnwal commented Aug 9, 2024

Closes #5433

  1. Adds time comparison lines for dimension line charts
  2. Adds time comparison bars for stacked and grouped bar chart.

@djbarnwal djbarnwal changed the title Feat/time comparison for dimensions Feat: Time comparison for dimensions charts Aug 25, 2024
@djbarnwal djbarnwal marked this pull request as ready for review August 25, 2024 13:45
@ericpgreen2
Copy link
Contributor

UXQA:

  1. Am I supposed to see the comparison lines at all times? I see them when I hover over the row of a specific dimension, but I don't see them otherwise, e.g.:
    image
  2. When hovering over different dimension rows, the tooltip from the prior row hangs. See the dot for "14" in this JAM.
  3. In the bar graph, I'd expect the comparison time range (typically earlier-in-time) would be to the left of the primary time range (typically later-in-time).
  4. On the bar graph, I find myself looking for a legend to understand what the different bars represent (CC @jkhwu)
  5. Because the Area chart does not represent the time comparison, I think I'd expect the time comparison to get toggled off.

@jkhwu
Copy link
Contributor

jkhwu commented Sep 5, 2024

Replying to @ericpgreen2's notes above

  1. This is per design. In an earlier iteration we showed comparison lines for all the colorful lines by default, but it got pretty messy looking and got quite hard to match comparison lines to their corresponding colored lines.
    Our ideal solution would be in addition to single compare lines appearing on table hover, to show a compare line when you're mousing over one line in the chart, however, @djbarnwal explained that it's quite difficult to detect which line the mouse is closest to in our current hand-rolled format (perhaps once we switch to Vega Lite this will be easier?)
    Right now the implementation is still missing the additional columns that will also appear when you turn on comparisons, so it's dissatisfying to flip the compare toggle and not see any changes to the page content. I think once the comparison columns show up, the table mouseover will be more discoverable and intuitive.

@jkhwu
Copy link
Contributor

jkhwu commented Sep 5, 2024

@ericpgreen2 cc @djbarnwal
3. I'm fine with switching the order of the bars for comparison vs current
4. The color coding is supposed to be contained within the checked rows of the table below, but I think better than a static legend would be to enhance the content of the hover tooltips to list multiple values at once. This has been requested by @ericokuma but I haven't followed up with a design proposal yet: https://www.notion.so/rilldata/Improving-Chart-Hover-Tooltips-45583322f8434cb98b1b05519dcd97a7?pvs=4
5. Once unblocked by platform adjustments, the table will still show comparison columns even if the selected viz type is area charts. I don't think we should auto-toggle comparisons off because of that.

@ericpgreen2
Copy link
Contributor

  1. This is per design.

@jkhwu, @djbarnwal what about showing the comparison lines when at most X specific dimension values are selected? Say X = 1, 2, or 3?

@jkhwu
Copy link
Contributor

jkhwu commented Sep 18, 2024

@ericpgreen2 , @djbarnwal will post a screenshot of what you're proposing and we'll assess the readability here.

@djbarnwal
Copy link
Member Author

Example 1
3 dimensions selected with lines close to each other

image

Example2
3 dimensions selected with well spaced lines
image

Personally I don't think it's adding much value here. To get a better look into the lines even in the spread out case, I have to hover over a row and inspect it.

@jkhwu
Copy link
Contributor

jkhwu commented Sep 24, 2024

@ericpgreen2 I agree with @djbarnwal's assessment. Based on the screenshots I think it's hard to tell which lines are current and which are comparison.

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

@ericpgreen2 I agree with @djbarnwal's assessment. Based on the screenshots I think it's hard to tell which lines are current and which are comparison.

Sounds good – approved 👍

@nishantmonu51
Copy link
Collaborator

@djbarnwal @ericpgreen2 : Whats the status here ? Are we able to merge this one ?

@djbarnwal djbarnwal merged commit fe07162 into main Nov 1, 2024
7 checks passed
@djbarnwal djbarnwal deleted the feat/time-comparison-for-dimensions branch November 1, 2024 22:28
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.

Enable simultaneous comparison by dimension and time in TDD charts
4 participants