[shared]: fix issue #167

[gnijuohz]

💔 Breaking Changes

🏆 Enhancements

📜 Documentation

🐛 Bug Fix

Fix issue #167, after this fix, a focus event won't show the tooltip anymore.

An alternative fix I tried was to add && event.type !== 'focus' to the condition here https://github.com/williaster/data-ui/blob/master/packages/shared/src/enhancer/WithTooltip.jsx#L71. In that case clicking on a bar for the first time (focusing it) would move the position of the bar. What do you think about this? @williaster

🏠 Internal

[codecov]

Codecov Report

Merging #171 into master will decrease coverage by 0.43%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
- Coverage   80.51%   80.07%   -0.44%     
==========================================
  Files         109      109              
  Lines        2422     2424       +2     
  Branches      568      569       +1     
==========================================
- Hits         1950     1941       -9     
- Misses        291      302      +11     
  Partials      181      181

Impacted Files	Coverage Δ
packages/shared/src/enhancer/WithTooltip.jsx	46.87% <0%> (-33.13%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7abd58...dd8ab83. Read the comment docs.

[codecov]

Codecov Report

Merging #171 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master     #171   +/-   ##
=======================================
  Coverage   80.51%   80.51%           
=======================================
  Files         109      109           
  Lines        2422     2422           
  Branches      568      568           
=======================================
  Hits         1950     1950           
  Misses        291      291           
  Partials      181      181

Impacted Files	Coverage Δ
packages/shared/src/enhancer/WithTooltip.jsx	80% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7abd58...b12d02d. Read the comment docs.

[williaster]

@gnijuohz thanks for the PR! I want to pull this and test locally to confirm that accessibility still works. Hopefully can merge in the next couple days.

[williaster]

Yeah sorry, this breaks tab-able tooltips entirely which was introduced as a requirement for accessibility.

However it looks like tab-ing doesn't work at all in firefox either, which is another bug in and of itself 😞 I'm a little confused, maybe we need another approach aside from wrapping in <a> tags 🤔

[gnijuohz]

Hi @williaster you're right. My previous fix prevents tooltips from showing up on focus. My updated fix should no longer break that.

[williaster]

Thanks @gnijuohz ! Pulled again to test and functionally it seems good to me (the movement of the tooltip on click doesn't seem super problematic to me)

[gnijuohz]

Thanks @williaster !

I haven't done much research on focusing elements in svg, what do you think about using tabindex? It seems to have great support from major browsers: https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/tabindex