victory icon indicating copy to clipboard operation
victory copied to clipboard

Safari Tooltip Incorrect Size Scaling in VictoryBar with VictoryTooltip

Open ledei opened this issue 11 months ago • 7 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

Victory version

"victory": "^37.0.0"

Code Sandbox link

Codesandbox

Bug report

Using the join("\n") method within the labels prop on VictoryBar incorrectly scales the displayed tooltip upon hovering, increasing significantly in size, to a much larger degree than expected. To the best of my findings this only seems to happen in Safari, when utilizing the VictoryBar component in conjunction with VictoryTooltip as its labelComponent.

Steps to reproduce

  1. Go to Codesandbox.
  2. Click on 'Open a new tab' Copy the url and paste it Safari.
  3. Hover over one of the upper piller.
  4. See error.

Expected behavior

The tooltip (VictoryTooltip) should maintain a consistent and appropriate size when hovered, similar to its behavior in other browsers, and should not excessively enlarge regardless of the amount of new lines in the label.

Google chrome: Skärmavbild 2024-03-18 kl  16 16 36 (2)

Actual behavior

Upon hover, the size of the tooltip (VictoryTooltip) expands disproportionately, far beyond the expected degree of enlargement. This issue has been noted in Safari, while not manifesting in Google Chrome, the only other browser tested thus far. However, it's important to acknowledge that this behavior may extend to other browsers that have not yet been examined.

Safari: Skärmavbild 2024-03-18 kl  16 16 44

Environment

  • Device: Desktop
  • OS: macOS Version 14.3.1
  • Browser: Safari

ledei avatar Mar 19 '24 10:03 ledei

Thanks for the bug report @ledei, I am able to reproduce this.

One thing to note is that /n is not valid syntax. To produce muti line labels, simply pass an array.

labels={() => ["this", "is", "a", "multiline", "label"]}

carbonrobot avatar Mar 19 '24 12:03 carbonrobot

Dev notes: In testing, it looks like our measurement utils are affected by this outstanding bug which causes Safari to return the bounds of the containing <text> element instead of the target <tspan> element. Possible solutions could be wrapping every <tspan> in a <text> element.

carbonrobot avatar Mar 19 '24 12:03 carbonrobot

Thank you so much for your quick response @carbonrobot! It's greatly appreciated.

Thanks for the bug report @ledei, I am able to reproduce this.

One thing to note is that /n is not valid syntax. To produce muti line labels, simply pass an array.

labels={() => ["this", "is", "a", "multiline", "label"]}

It looks like I'm facing a similar issue when I send an array instead of a string with /n. Is there anything that I can do to move forward on this?

ledei avatar Mar 19 '24 16:03 ledei

I'm encountering a similar issue with VictoryVoronoiContainer on Safari.

You can reproduce the issue with the existing voronoiDimension example. As you add more data sets, the tooltip grows exponentially. I'm using 4 VictoryScatter components in the example below.

Screenshot 2024-05-13 at 3 52 43 PM

dlabrecq avatar May 13 '24 19:05 dlabrecq

Are there any updates on this? I am also encountering this issue

jane-airen avatar Jul 18 '24 06:07 jane-airen

I think I might be experiencing this behaviour - I am struggling meaningfully to debug it. I have a custom VoronoiZoom component:

<VictoryChart
  width={1000}
  height={800}
  style={styles.chartMisc}
  domain={computedDomains}
  containerComponent={
      <VictoryZoomVoronoiContainer
          data-testid="label-container"
          containerRef={ref => { chartRef.current=ref} }
          allowZoom={allowZooming}
          allowPan={allowZooming}
          onZoomDomainChange={handleZoomChange}
          zoomDomain={domains}
          labels={({ datum }) => {
              // This the tool tip text, and accepts a large number of arguments
              // tool tips return contextual information for each datapoint, as well as the centile
              // and SDS lines, as well as bone ages, events and midparental heights
              const tooltipTextList = tooltipText(
                  reference,
                  measurementMethod,
                  datum,
                  midParentalHeightData,
                  clinicianFocus,
                  sex
              )
              if (tooltipTextList){
                  return tooltipTextList.join('\n').replace(/^\s+|\s+$/g, '');
              } 
          }}
          labelComponent={
              <VictoryTooltip
                  data-testid='tooltip'
                  constrainToVisibleArea
                  backgroundPadding={5}
                  pointerLength={5}
                  cornerRadius={0}
                  flyoutStyle={{
                      ...styles.toolTipFlyout,
                  }}
                  style={{...styles.toolTipMain}}
              />
          }
          voronoiBlacklist={['linkLine', 'chronologicalboneagelinkline', 'correctedboneagelinkline', 'areaMPH']}
      />
  }
>

The tooltipText function called in the labels element of the VictoryZoomVoronoiContainer returns an array of strings but if I return the array as @ledei suggests I simply get a long single line tooltip with the strings separated by commas, rather than than a multiline tooltip. I therefore add \n elements at the end of each line and strip the ones at the beginning and the end.

In chrome I get expected behaviour: image but in Safari I get: image this is an improvement on what I had before when I was returning a long concatenated string with /n interspersed for the line breaks: image

If you have any workarounds to suggest I would be super grateful.

eatyourpeas avatar Sep 08 '24 23:09 eatyourpeas

Update, just in case it helps others. By taking advantage of the flyoutHeight prop it is possible to use the lineheight of the text to set the height of the tooltip.

...
labelComponent={
  <VictoryTooltip
      data-testid='tooltip'
      constrainToVisibleArea
      backgroundPadding={5}
      pointerLength={5}
      cornerRadius={0}
      flyoutHeight={(datum) => {
          const numberOfLines = datum.text.length;
          return numberOfLines * 18;    // 18 is the line height
      }}
      flyoutStyle={{
          ...styles.toolTipFlyout,
      }}
      style={{...styles.toolTipMain}}
  />
}
...

eatyourpeas avatar Sep 10 '24 07:09 eatyourpeas