General navigable component #3303

ephox-mogran · 2017-11-02T01:27:47Z

Description

This is based on the work from #3281. I'm just pulling out the NavigableContainer part and adding a README and some data tests (and some skipped UI tests).

How Has This Been Tested?

Manually, and there are some automated tests for the data transformations and movement.

Screenshots (jpeg or gifs if applicable):

Types of changes

Replaced an existing component: NavigableMenu. There is one change in the functionality: it no longer handles tab presses. We might want to reinstate this functionality, and if we do, we should make that optional ... as not all NavigableMenus are necessarily going to want to handle tab.
Created a sort of abstract component called NavigableContainer, which is exposed as:
- NavigableMenu
- NavigableGrid
- TabbableContainer

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows has proper inline documentation.

ephox-mogran · 2017-11-02T05:47:15Z

This is then used in #3281

youknowriad

Awesome job here!!

I left some comments, I'm trying to minimize the external API as much as possible. (which is generally good for maintenance) and I'm wondering if the "mode" abstraction is necessary.

But overall, this is great.

youknowriad · 2017-11-02T14:27:37Z

components/navigable-container/README.md

+
+### initialSelector
+
+A selector which tells the component what to focus first inside it. If the selector is not matched, the first focusable item will be focused. This can be useful to focus the previously selected descendant when this component get focused.


I tend to avoid "selector" props because they break quickly without noticing.
Is there a way we could remove this prop entirely, say by always focusing the first tabbable or something?

--

Alternatively, we could try to always fallback to the tabbable with aria-selected if present

I've liked being able to use classes for things which have 'fake focus' in the past, but it's easy enough to add later I suppose if we need it. The aria-selected selector is definitely currently needed in TabPanel (you don't want to just focus the first tab if one was previously selected), so I guess we could look for just that first and fallback to the first one as you say. I tend to over-engineer rather than under-engineer, so you're just seeing that excess :)

youknowriad · 2017-11-02T14:28:56Z

components/navigable-container/README.md

+
+### widget
+
+A boolean which specifies whether or not the focusable elements in this component might contain the focus, rather than have the focus. This is often used for `TabbableContainer` layouts that might have arrow key navigation inside each tabstop.


Not sure I understand, could you clarify a bit?

Yep, I'll clarify here, and then it makes sense here, I'll (try to remember to) update the readme.

The way this component is setup is designed to simplify composing different navigation models. Because it's the example in the other PR, it's easiest to to use the InserterMenu as an example.

The InserterMenu has a tabstop at the row of tabs, and a tabstop at the grid for each category, and a tabstop for the searcher. Now, the outer container of the inserter menu is just a TabbableContainer, with only knowledge of the three tabstops. However, once the row of tabs gets focused (for example), its own navigation activates. The row of tabs is a NavigableMenu, so it focused the selected tab inside.

Now, if the user presses tab, the NavigableMenu doesn't handle that, so it safely bubbles to the TabbableContainer. Now, the tabbablecontainer has to work out which of its tabstops currently has focus. But none of its tabstops currently have focus. A button which is inside one of its current tabstops has focus, but the tabstop itself doesn't. That's because the tabstop for the row of tabs is a widget.

It's part of the purpose of these modes. I've written something very similar to this before in a UI library I've made previously, so I'm probably guilty of trying to replicate something I feel works fairly well. Navigation often needs to be composable. Something on the outside tabs in between various tabtops, and those tabstops themselves have arrow key navigation (is the common example). Some things are even more complex. By allowing for different modes, you make it possible to compose different types of navigation within each other, and allow bubbling of unhandled keys (by a particular navigation mode) pass the keys to the outer navigation layer which does care about those keys. Once you set it up this way, all types of navigation become fairly simple to implement. If you discover that you need to handle ENTER or ESCAPE or something similar, or different types of keys for switching focus (HOME and END), then you just have to either add modes, or add configuration to a mode.

I realised I just tried to explain what modes are for. Does any of that make sense?

youknowriad · 2017-11-02T14:31:29Z

components/navigable-container/README.md

+- Required: No
+- Default: `"vertical"`
+
+#### stopArrowKeys


Can we find a better name? preventBubblingArrowEvents or something. I'm bad at naming things though :)

That name sounds slightly better :)

youknowriad · 2017-11-02T14:34:19Z

components/navigable-container/README.md

+
+### width
+
+An integer specifying the number of components that are displayed across the screen in one row. This relates to how the arrow keys will determine what to focus next.


My personal preference would be to drop this in favor of having the right behave exactly as down...
What if we have incomplete rows, this width prop won't be sufficient to ensure the right behavior

This particular mode is designed to only handle a situation where the grid is laid out visually in rows (due to wrapping) but there is no DOM structure that enforces those rows (so it's essentially just CSS). I found that when playing with the InsertMenu in the blocks section, pressing down and seeing the focus shift to the right was very disconcerting, so this is just to allow a nice clean navigation for grids with CSS wrapping. The problem is that you either need to calculate the width at run-time (via y position changes), or know it in advance (which you might know depending on how your CSS is making it show in different rows).

This code is designed to work for that situation for a regular grid. The regular grid can have fewer columns on its last row, but the last row is the only one that can be different. It is basically an exact match for what I was seeing with the block categories on the Inserter Menu. We've used it in the past for emoticon panels etc. Users tend to expect to be able to navigate across a grid with the arrow keys.

Having said all that, if you do want to avoid doing this, and only use grid navigation for situations when each row is a new DOM structure (like a table), then that makes sense. It means there is a lot less guesswork, obviously.

We had this kind of navigation in the firsts iterations of the Inserter, but for some reason, it was reverted to a simpler alternative. Shame that I don't recall the reason and I couldn't find the PR responsible for that neither :(

I've reverted it to just use down to the move to the right.

youknowriad · 2017-11-02T14:44:40Z

components/navigable-container/modes.js

+			return {
+				...pick( tabConfig, [ 'deep', 'widget', 'initialSelector' ] ),
+				useTabstops: true,
+				detect: ( event ) => {


I'm not a big fan of this "mode" abstraction, Do you think we could remove it entirely in favor of something like:

onKeyDown( event ) { const offset = eventToOffset( event ) // check if we use tabs or arrows and returns 1 or -1 or null if ( ! offset ) { return; } const tabbables = getTabbables() // depending on the deep prop const currentIndex = tabbables.indexOf( document.activeElement) const nextIndex = cycle( current, tabbables.lenght, offset ) // apply the cycle prop if necessary tabbables[ nextIndex ].focus(); }

I'm finding that the "mode" abstraction adds indirection where it's not necessary.

Yeah, this will not support any kind of composition of keyboard navigation, and will not allow grid navigation. Maybe grid navigation isn't as important as I thought it was, but it certainly 'feels' nicer. My recommendation is to play with the InsertMenu PR (#3281) and see what I'm trying to do as a whole. That will give you a better feel about whether this level of indirection is worthwhile. The answer may definitely be "no, not really" :)

Why can't we have something like this while keeping the widget prop? As I understand it, it applies when trying to get the currentIndex, so instead of const currentIndex = tabbables.indexOf( document.activeElement) we check the prop and use contains if it's a widget?

Also, I wonder if we should rename the prop. widget might not be explicit enough, but It's hard to find a good name. maybe navigateContainers (I don't really know, just thinking out loud).

Not saying the current implementation is bad, it's not, it's great but you know, I like simple functions ;). I find that if we can get this, it makes it clear, how each prop changes the behavior instead of having to dig to calculateMode and then get back to the component to understand.

Hmm. Let's explore this.

So mode is responsible for these details:

deep: used by navigable container
widget: used by navigable container
initialSelector used by navigable container, but as you said, we can just make it aria-selected until it is required and drop this altogether
useTabstops used by navigable container to identify if looking for a tabindex of 0, or any tabindex
detect: used by navigable container to determine the next index based on keycodes

Note: doing this has shown me that I've got the wrong shape for the fallback return. detect used to be an array of rules :). I should now return a detect: () => null rather than rules: [ ].

Most of these values (cycle, deep etc.) are passed through as is (though some might have different default values). One minor part that mode is providing is working out whether the focusable targets have a tabindex of -1 (programmatic focus only) or 0 (normal tab focus) via the poorly named useTabstops prop. The major part that mode is providing is the detect function. The detect function has two primary roles:

a. Works out which keys are listened to. This is important. If you don't listen to a key (or stop it), then you enable your ancestors to listen to it, which can help with composing keyboard navigation. E.g. I may not care about tab, but my ancestor might.

b. Works out how to change the index based on which key was pressed. This is particularly important if you want down to have a different meaning from right. Or allow home to jump straight to index 0 and end straight to total - 1. This home and end capability isn't implemented now, but is mentioned as a possible improvement in the ARIA guidelines for navigating tabs (https://www.w3.org/TR/wai-aria-practices/examples/tabs/tabs-1/tabs.html)

If you wanted to get rid of mode, AND get rid of any potential for any keycode to impact the index by more than +1 or -1, then you would be able condense this down as you suggested. You would lose future flexibility, but the code would be simpler.

If you wanted to get rid of mode, AND have the ability to have more flexible index repositioning with focus (support home, end, down, up etc) then you have two options:

a) provide the detect function to NavigableContainer as a prop, and provide useTabstops as a prop.
b) provides a list of keycodes and the index changes they make (originally how rules worked earlier in the PR), and provide useTabstops as a prop

Essentially, this would be recreating modes. In (b)'s case, it would probably create more duplication, and it would force people to constantly recreate exactly the same key rules each time (right means +1, left means -1, down means +x, up means -x, home means 0, end means length - 1), and handle cycling themselves for non-trivial cases. All modes are trying to do is encompass common types of navigation, so that they can be reused easily. I can see how they can be slightly harder to follow, though.

My preference is to stay with modes, but I can also see how a above (passing detect through) would make the code easier to read. I'd be reluctant to fall back to just changing the offset by -1 or +1 depending on whether the key was a 'backward' (left, up) key or a 'forward' (right, down) key --- but maybe my prior experience with more complex navigation requirements is not relevant to Gutenberg.

I'm not sure if we'll be able to convince each other of anything without running code, so it might just be simpler if you alter this to look the way that you want, and we can then compare the advantages and disadvantages of each approach. Does that sound productive?

Although, since the code has been refactored since the initial attempt, the calculateMode code could certainly be done in the exported components themselves (rather than passing in an object for NavigableContainer to transform). Originally, NavigableContainer was going to be exported directly.

I see two main issues with the detect function:

It obscures simpler props

Composition is made more difficult, thus we loose flexibility instead of gaining flexibility IMO which means If I have a mode, that combines "tabs", "grid" and it's not provided by calculateMode it's hard to do unless you pass the detect function like you explained, but the detect function is not trivial and it can't be an external API of the component IMO.

About the -1/1 value, it was just a simplification because I didn't think we need the grid navigation right now, but if we need it, we can always change the return value of eventToOffset to accommodate these usecases. ({ x: 1, y: 2})

That won't handle home and end jumping to the start and end respectively. But if you're worried about having functions in the API, and you don't want modes, then I guess there isn't any alternative.

ephox-mogran · 2017-11-02T22:47:59Z

Also, the current plan is not to expose NavigableContainer and only expose TabbableContainer, NavigableMenu and NavigableGrid. Therefore, mode is won't be part of the API surface. It's an implementation detail (to some extent).

ephox-mogran · 2017-11-03T04:59:47Z

Interestingly, we can get the focus tests to work in jsdom, as long as we override the getClientRects() method of the DOM elements themselves. The problem as the comment suggests is that the isVisible check in focusable fails, and it fails because jsdom doesn't have a layout engine. Therefore, everything is given a size of 0. With something similar to:

function simulateVisible( wrapper, selector ) {
	const elements = wrapper.getDOMNode().querySelectorAll( selector );
	each( elements, ( elem ) => {
		elem.getClientRects = () => [ 'trick-jsdom-into-having-size-for-element-rect' ];
	} );
}

You can get the focus tests to work (the ones skipped in this PR and the NavigableMenu on master). Is that a good approach do you think?

youknowriad · 2017-11-03T09:14:22Z

Is that a good approach do you think?

Adding this mock is definitely a good idea to avoid skipping the tests 👍

ephox-mogran · 2017-11-07T06:56:51Z

@youknowriad I've simplified this trying to implement what you are tasking for. I've also updated the #3281 to use this updated version.

I haven't updated the README yet, because I'd rather see if you're happy with this approach first ( / ran out of time :) )

youknowriad · 2017-11-07T07:29:47Z

components/navigable-container/index.js

+
+		const offset = eventToOffset( event );
+
+		const stopNavigationKey = ( preventBubblingArrowEvents && [ LEFT, RIGHT, DOWN, UP ].indexOf( keyCode ) > -1 ) ||


Could this be simplified like preventBubblingArrowEvents && offset !== 0

Oh I see, it's relying on preventBubblingTabEvents or preventBubblingArrowEvents which make things a bit different. Can't we use the same prop? preventBubblingNavigationEvents?

They can't use the same prop. Think of the Inserter Example

TabbingCycle

Input

TabList (cares about arrows)

tab1

tab2

TabView (cares about arrows)

item1

item2

If the TabList (which needs to stop arrow keys bubbling also stopped tab keys bubbling), then the TabbingCycle wouldn't work. The TabbingCycle needs to receive the tab key, otherwise it can't force tab to cycle within itself. They need to be separate. Does that make sense?

I've had to rename the prop because we need to preventDefault in many situations as well, not just stopPropagation.

youknowriad · 2017-11-07T07:33:55Z

components/navigable-container/index.js

+
+export class NavigableMenu extends Component {
+	render() {
+		const { deep = true, cycle = true, orientation = 'vertical', ...rest } = this.props;


Can we drop deep and cycle from here, and rely on ...props to pass them down. Same for TabbableContainer

Yep. I think this was leftover from when there were different defaults (or information was needed for modes).

youknowriad

This looks pretty solid to me. Thanks for the hard work. I left some small notes but we're pretty close to the merge

ephox-mogran · 2017-11-08T04:26:59Z

@youknowriad anything else?

youknowriad · 2017-11-08T08:41:57Z

components/navigable-container/README.md

+- Required: No
+- default: true
+
+## handleRef


Can you clarify why we need this prop. I'd love if we can avoid it since it's too "technical".

youknowriad · 2017-11-08T08:42:48Z

components/navigable-container/README.md

+			<div tabIndex="0">Section 1</div>
+			<div tabIndex="0">Section 2</div>
+			<div tabIndex="0">Section 3</div>
+      <div tabIndex="0">Section 4</div>


mixed space/tabs indentation

youknowriad · 2017-11-08T08:45:33Z

components/navigable-container/index.js

@@ -146,22 +139,21 @@ export class NavigableMenu extends Component {
 			return 0;
 		};

-		return <NavigableContainer deep={ deep } cycle={ cycle } eventToOffset={ eventToOffset } { ...rest } />;
+		return <NavigableContainer stopArrowEvents={ true } stopTabEvents={ false }


Minor, you can drop the ={ true } for all props with a true value.

youknowriad · 2017-11-08T08:51:30Z

components/navigable-container/stoppers.js

+
+function handleHorizontalArrows( event ) {
+	// Do not preventDefault if inside a textarea or input where the arrow keys should move inside it.
+	if ( ! [ 'textarea', 'input' ].indexOf( event.target.nodeName.toLowerCase() ) ) {


What about contenteditable.

I probably came to the conclusion that you did below that no-one would want a contenteditable inside an arrowable region. Having said that, I have no idea why I thought textarea and input were more likely :)

youknowriad · 2017-11-08T08:54:13Z

components/navigable-container/stoppers.js

+
+	if ( stopArrowEvents && [ LEFT, RIGHT ].indexOf( keyCode ) > -1 ) {
+		handleHorizontalArrows( event );
+	} else if ( stopArrowEvents && [ UP, DOWN ].indexOf( keyCode ) > -1 ) {


Why UP/DOWN are different than LEFT/RIGHT both need to move cursor in inputs/textarea right?

Yep. Just confused myself. Up and down you don't typically use in a single line input, but you certainly would in a textarea.

youknowriad · 2017-11-08T08:58:39Z

components/navigable-container/stoppers.js

+	} else if ( stopTabEvents && TAB === keyCode ) {
+		stopEvent( event );
+	}
+}


Thinking about this more. I'm wondering why we're not systematically preventing and stopPropagation consistently.

I mean if we're using arrows in a navigable menu, we expect these arrows to navigate to other elements, thus, we need to preventEverything. And if we're using tabs same.

By this I mean, it makes no sense to me to use arrow navigation if your children could contain inputs/textareas...

which brings us to my simple check: preventBubblingEvents && offset !== 0. This would work because if use arrows, the offset will be different than 0 only for arrow keys and same for tabs.

Hmm. Interesting idea. I have no problem with this but it would mean that the person writing the offset function would need to understand that returning 0 for an offset, even if it is just because they have hit the edge, means that the event will not be handled and will bubble up (which causes problems when things like WritingFlow listen to arrow keys above). I guess given that cycle is done here (and not done by the offset calculation code which could return 0 at the edges), then the offset function will just have to be clear that you do not return 0 unless you want the event to bubble.

My personal style is to avoid conflating stopping the event and moving the focus, but I can see the benefits (ensure the two are kept in sync). This is done most easily by having a complicated function, so I can see why that is undesirable in the API. Your approach will work if you work on the assumption that offset 0 means do not bubble.

Another problem with this is that it means that for situations where you have a horizontal menu (left, right arrow keys), pressing up and down will have an offset of 0, so they will bubble to WritingFlow and cause problems. Initially, I introduced this so that even the arrow keys that weren't involved in moving the focus were being stopped. Otherwise, the other keys (up instead of left) etc. are going to bubble.

As discussed in DM, your idea of returning undefined for do not stop this event, and 0 for stop it, but don't move seems good to me.

codecov · 2017-11-08T10:28:15Z

Codecov Report

Merging #3303 into master will increase coverage by 0.6%.
The diff coverage is 93.65%.

@@            Coverage Diff            @@
##           master    #3303     +/-   ##
=========================================
+ Coverage   31.34%   31.94%   +0.6%     
=========================================
  Files         246      246             
  Lines        6779     6814     +35     
  Branches     1220     1232     +12     
=========================================
+ Hits         2125     2177     +52     
+ Misses       3914     3906      -8     
+ Partials      740      731      -9

Impacted Files	Coverage Δ
components/dropdown-menu/index.js	`75% <ø> (ø)`	⬆️
editor/block-settings-menu/index.js	`0% <0%> (ø)`	⬆️
components/navigable-container/index.js	`95.16% <95.16%> (ø)`

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 eab4b64...2f6f1c8. Read the comment docs.

youknowriad

Nice work

aduth · 2017-11-15T20:11:57Z

components/navigable-container/index.js

+		const { deep = false, onlyBrowserTabstops } = this.props;
+		const finder = onlyBrowserTabstops ? focus.tabbable : focus.focusable;
+		const focusables = finder
+			.find( this.container )


Array#find is not polyfilled and is not supported by IE11. This results in crashes in IE11 (e.g. opening inserter).

Nevermind, this is the focus utility find method 🤦‍♂️ There's still an error, but I think it's caused by https://github.com/WordPress/gutenberg/pull/3281/files#r151239536.

aduth · 2018-01-08T16:51:07Z

components/navigable-container/index.js

+					'onlyBrowserTabstops',
+				] ) }
+				onKeyDown={ this.onKeyDown }
+				onFocus={ this.onFocus }>


Is this not doing anything?

Edit: And if it's not, should it be? (i.e. updating offset from receiving focus)

aduth · 2018-01-08T16:55:22Z

components/navigable-container/index.js

+		const { children, ...props } = this.props;
+
+		// Disable reason: Assumed role is applied by parent via props spread.
+		/* eslint-disable jsx-a11y/no-static-element-interactions */


This rule is disabled but then never turned on again (meaning it will take effect for the remainder of the file).

ephox-mogran requested a review from youknowriad November 2, 2017 01:27

ephox-mogran mentioned this pull request Nov 2, 2017

Using NavigableContainer to make a TabPanel for Inserter #3281

Merged

3 tasks

youknowriad reviewed Nov 2, 2017

View reviewed changes

youknowriad reviewed Nov 7, 2017

View reviewed changes

youknowriad reviewed Nov 8, 2017

View reviewed changes

ephox-mogran added 16 commits November 8, 2017 11:28

navigable container with basic README and skipped tests

ce0a7d5

adding more tests for modes

9297100

grid navigation for irregular grids

4dc7ed2

fixing id in skipped test

e12b95c

replacing other NavigableMenu

3c0191e

using NavigableMenu throughout app

68e08cf

whitespace

4f9d113

accessing DOM (but tests are skipped anyway)

aa9c7d5

readme fixes and a bit of cleanup

4b7d053

simplifying context detection

972b921

removing initial selector

91d5667

refactoring getInitialFocus

f3a2a00

Removing initial selector logic altogether

e077903

remove modes

81de0f2

noop for onNavigate

3e20a76

handling tab properly

3627437

ephox-mogran and others added 7 commits November 8, 2017 11:30

Updated readme, tests

50e08db

merged with master, and allowing role to be passed in

257475a

more comprehensive testing

599ae60

unused import

7c417cc

removing incorrect export

fc78bf5

Components: Simplify navigable container stopping events prop

b95fbf5

NavigableContainer: Dropping the handleRef prop

2f6f1c8

youknowriad force-pushed the try/general-navigable-component branch from 3096e3a to 2f6f1c8 Compare November 8, 2017 10:30

youknowriad approved these changes Nov 8, 2017

View reviewed changes

youknowriad merged commit 753394a into master Nov 8, 2017

youknowriad mentioned this pull request Nov 8, 2017

Toolbar: consider to use a roving tabindex and standardize navigation #3383

Closed

mtias deleted the try/general-navigable-component branch November 14, 2017 14:00

aduth reviewed Nov 15, 2017

View reviewed changes

aduth reviewed Jan 8, 2018

View reviewed changes

This was referenced Jun 22, 2018

Block More Options menu: arrows navigation doesn't work any longer #7443

Closed

Fix arrows navigation in the block more options menu #7480

Merged


		### initialSelector

		A selector which tells the component what to focus first inside it. If the selector is not matched, the first focusable item will be focused. This can be useful to focus the previously selected descendant when this component get focused.


		### widget

		A boolean which specifies whether or not the focusable elements in this component might contain the focus, rather than have the focus. This is often used for `TabbableContainer` layouts that might have arrow key navigation inside each tabstop.


		### width

		An integer specifying the number of components that are displayed across the screen in one row. This relates to how the arrow keys will determine what to focus next.


		const offset = eventToOffset( event );

		const stopNavigationKey = ( preventBubblingArrowEvents && [ LEFT, RIGHT, DOWN, UP ].indexOf( keyCode ) > -1 ) \|\|

General navigable component #3303

General navigable component #3303

Conversation

ephox-mogran commented Nov 2, 2017

Description

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

ephox-mogran commented Nov 2, 2017

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ephox-mogran Nov 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ephox-mogran commented Nov 2, 2017

ephox-mogran commented Nov 3, 2017

youknowriad commented Nov 3, 2017

ephox-mogran commented Nov 7, 2017

youknowriad Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

ephox-mogran commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 8, 2017 • edited Loading

Codecov Report

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ephox-mogran Nov 2, 2017 •

edited

Loading

youknowriad Nov 7, 2017 •

edited

Loading

youknowriad Nov 8, 2017 •

edited

Loading

youknowriad Nov 8, 2017 •

edited

Loading

codecov bot commented Nov 8, 2017 •

edited

Loading

aduth Jan 8, 2018 •

edited

Loading