Skip to content

Web [Charts]: Fix wrong rendering order and focus/hover line state

Warren Gifford requested to merge vk/old-chart-line-fix into main

Created by: vovakulikov

Fixes: https://github.com/sourcegraph/sourcegraph/issues/38427 Fixes: https://github.com/sourcegraph/sourcegraph/issues/39789

Background

Imagine that you have a line chart with two lines on it, with five dots in each line. And lines are located close enough to each other, so close that it is usually impossible to aim at or see any of lines points behind other lines.

Untitled-2022-08-06-1827-2

So we have to change the z-index somehow for SVG elements to pull hovered/focused line to the front. According to the SVG spec, we don't have any control over elements order except the actual DOM elements order.

3.3 Rendering Order Elements in an SVG document fragment have an implicit drawing order, with the first element in the SVG document fragment getting "painted" first. Subsequent elements are painted on top of previously painted elements.

Based on this, we need to change SVG DOM elements' order when we hover points on the chart. But if we're changing elements order on some trigger (mouse hover or keyboard focus navigation), we're breaking the normal focus management for the chart.

Let's imagine you're focusing on the first point on the chart. We immediately change the order in a way to bring this line on top (this means this line now is the last one in the chart according to the DOM hierarchy). This means that when you end focus this line, the focus naturally will go away from the chart since you're going from the last element on the chart already.

Solution

Instead of changing the order of chart lines (bars, arcs, whatever), we will render the line copy as the last element on the chart (so visually, it will be over all other elements on the chart) and mark this as hidden for keyboard and all a11y technologies). In this case, we don't break anything about focus management for the SVG elements.

<svg>
  <line .../>
  <!-- Focus is somewhere in the second line -->
  <line .../> 
  <line .../>

   <!-- Copy of the second line element that it's rendered as the last element  -->
  <line tabIndex="-1" pointer-events="none" aria-hidden="hidden".../> 
</svg>

Test plan

  • Check that focus works properly for line charts with more than one series (lines)
  • Check that multi lines charts on the code insight dashboard page (or analytics pages) don't have a weird z-index problem (on the initial mount and when you're hovering/focusing different lines on the chart)

App preview:

Check out the client app preview documentation to learn more.

Merge request reports