new(xychart): add support for FocusEvents #959
Conversation
…o bar+glyph series
| @@ -5,18 +5,18 @@ import { AxisScaleOutput } from '@visx/axis'; | |||
| import { ScaleConfig } from '@visx/scale'; | |||
|
|
|||
| import DataContext from '../context/DataContext'; | |||
| import { Margin, PointerEventParams } from '../types'; | |||
| import { Margin, EventHandlerParams } from '../types'; | |||
There was a problem hiding this comment.
re-named since no longer specific to PointerEvents
| @@ -7,6 +7,6 @@ export default function AnimatedAreaSeries< | |||
| XScale extends AxisScale, | |||
| YScale extends AxisScale, | |||
| Datum extends object | |||
| >({ ...props }: Omit<BaseAreaSeriesProps<XScale, YScale, Datum>, 'PathComponent'>) { | |||
| >(props: Omit<BaseAreaSeriesProps<XScale, YScale, Datum>, 'PathComponent'>) { | |||
There was a problem hiding this comment.
these were all double spread unnecessarily
Pull Request Test Coverage Report for Build 406606163
💛 - Coveralls |
| @@ -65,6 +66,7 @@ export default function AnimatedBars<XScale extends AxisScale, YScale extends Ax | |||
| item == null || key == null ? null : ( | |||
| <animated.rect | |||
| key={key} | |||
| tabIndex={isFocusable ? 0 : undefined} | |||
There was a problem hiding this comment.
noting that this implementation requires the SVG 2.0 spec – SVG 1.1 doesn't support tabIndex which means this may not work on older browsers. the alternative approach (used in data-ui) was to wrap each element in an <a /> element, which are consistently tab-able by all browsers.
For now I think this more modern approach is more ideal since it's simpler and it is supported across all modern browsers
| onPointerMove, | ||
| onPointerOut, | ||
| onPointerUp, | ||
| enableEvents = true, |
There was a problem hiding this comment.
pointerEvents => enableEvents throughout
| source: string; | ||
| }; | ||
|
|
||
| /** This hook simplifies the logic for initializing Series event emitters + handlers. */ |
There was a problem hiding this comment.
all of this boilerplate was repeated in each *Series previously
| const getNumericValue = <XScale extends AxisScale, YScale extends AxisScale>( | ||
| bar: BarStackDatum<XScale, YScale>, | ||
| ) => getSecondItem(bar); // corresponds to y1, the upper value (topline). | ||
| ) => (getFirstItem(bar) + getSecondItem(bar)) / 2; |
There was a problem hiding this comment.
when only accounting for the top line of a bar, logic for finding the nearest series datum to the middle of the bar bounding box was incorrect
🚀 Enhancements
This PR builds on #947 and adds support for
FocusEvents to all series via theonFocusandonBlurhandlers. This mostly involveduseSeriesEventshook to simplify a lot of the boilerplatefindNearestDatumlogic in event handlers, this is needed forBarGroupandBarStackTests coming in another PR.
🐛 Bug Fix
BarStackSerieswas lost in new(xychart): add PointerEvent handlers to XYChart and *Series #947 here@kristw @hshoff