From bf863c3922e95bed8f1c3f5829acf2ff6724e647 Mon Sep 17 00:00:00 2001 From: Sara Valderrama Date: Fri, 10 Aug 2018 14:52:56 -0700 Subject: [PATCH] Logging added for ax mode and updated to fix console warnings Summary: Added logging for accessibility functionality (both usage and performance). Fix to prevent trackTimeSince calls from not matching up to the correct marks. Fix to prevent recursive onElementExpanded calls from mismatching also. Reviewed By: danielbuechele Differential Revision: D9229790 fbshipit-source-id: d20f08719d2c4f9a35c9c71a492619ce5538d204 --- .../inspector/InspectorSonarPlugin.java | 5 + .../descriptors/ApplicationDescriptor.java | 8 +- src/plugins/layout/index.js | 104 ++++++++++++++---- 3 files changed, 95 insertions(+), 22 deletions(-) diff --git a/android/src/main/java/com/facebook/sonar/plugins/inspector/InspectorSonarPlugin.java b/android/src/main/java/com/facebook/sonar/plugins/inspector/InspectorSonarPlugin.java index 9f721badd..bc22cd90a 100644 --- a/android/src/main/java/com/facebook/sonar/plugins/inspector/InspectorSonarPlugin.java +++ b/android/src/main/java/com/facebook/sonar/plugins/inspector/InspectorSonarPlugin.java @@ -391,6 +391,11 @@ public class InspectorSonarPlugin implements SonarPlugin { // if in layout inspector and talkback is running, override the first click to locate the clicked view if (mConnection != null && AccessibilityUtil.isTalkbackEnabled(getContext()) && event.getPointerCount() == 1) { + SonarObject params = new SonarObject.Builder() + .put("type", "usage") + .put("eventName", "accessibility:clickToInspectTalkbackRunning").build(); + mConnection.send("track", params); + final int action = event.getAction(); switch (action) { case MotionEvent.ACTION_HOVER_ENTER: { diff --git a/android/src/main/java/com/facebook/sonar/plugins/inspector/descriptors/ApplicationDescriptor.java b/android/src/main/java/com/facebook/sonar/plugins/inspector/descriptors/ApplicationDescriptor.java index 5e34d4ff4..4b713c72d 100644 --- a/android/src/main/java/com/facebook/sonar/plugins/inspector/descriptors/ApplicationDescriptor.java +++ b/android/src/main/java/com/facebook/sonar/plugins/inspector/descriptors/ApplicationDescriptor.java @@ -75,7 +75,8 @@ public class ApplicationDescriptor extends NodeDescriptor { for (View view : node.getViewRoots()) { // unlikely, but check to make sure accessibility functionality doesn't change - if (view instanceof ViewGroup && !ViewCompat.hasAccessibilityDelegate(view)) { + boolean hasDelegateAlready = ViewCompat.hasAccessibilityDelegate(view); + if (view instanceof ViewGroup && !hasDelegateAlready) { // add delegate to root to catch accessibility events so we can update focus in sonar view.setAccessibilityDelegate(new View.AccessibilityDelegate() { @@ -107,6 +108,11 @@ public class ApplicationDescriptor extends NodeDescriptor { } }); editedDelegates.add((ViewGroup) view); + } else if (hasDelegateAlready) { + SonarObject params = new SonarObject.Builder() + .put("type", "usage") + .put("eventName", "accessibility:hasDelegateAlready").build(); + mConnection.send("track", params); } } } diff --git a/src/plugins/layout/index.js b/src/plugins/layout/index.js index 882ebb6e5..807f34ffb 100644 --- a/src/plugins/layout/index.js +++ b/src/plugins/layout/index.js @@ -25,6 +25,8 @@ import { VerticalRule, } from 'sonar'; +import type {TrackType} from '../../fb-stubs/Logger.js'; + import { AXElementsInspector, AXToggleButtonEnabled, @@ -50,6 +52,7 @@ export type InspectorState = {| inAXMode: boolean, AXtoNonAXMapping: {[key: ElementID]: ElementID}, isAlignmentMode: boolean, + logCounter: number, |}; type SelectElementArgs = {| @@ -93,6 +96,12 @@ type GetNodesOptions = {| forFocusEvent?: boolean, |}; +type TrackArgs = {| + type: TrackType, + eventName: string, + data?: any, +|}; + type SearchResultTree = {| id: string, isMatch: Boolean, @@ -184,6 +193,7 @@ export default class Layout extends SonarPlugin { AXfocused: null, AXtoNonAXMapping: {}, isAlignmentMode: false, + logCounter: 0, }; reducers = { @@ -480,15 +490,22 @@ export default class Layout extends SonarPlugin { } initAX() { + performance.mark('InitAXRoot'); this.client.call('getAXRoot').then((element: Element) => { this.dispatchAction({elements: [element], type: 'UpdateAXElements'}); this.dispatchAction({root: element.id, type: 'SetAXRoot'}); this.performInitialExpand(element, true).then(() => { + this.props.logger.trackTimeSince('InitAXRoot', 'accessibility:getRoot'); this.setState({AXinitialised: true}); }); }); this.client.subscribe('axFocusEvent', ({isFocus}: AXFocusEventResult) => { + this.props.logger.track('usage', 'accessibility:focusEvent', { + isFocus, + inAXMode: this.state.inAXMode, + }); + // if focusing, need to update all elements in the tree because // we don't know which one now has focus const keys = isFocus ? Object.keys(this.state.AXelements) : []; @@ -526,8 +543,15 @@ export default class Layout extends SonarPlugin { ); this.client.subscribe('selectAX', ({path}: {path: Array}) => { + if (this.state.inAXMode) { + this.props.logger.track('usage', 'accessibility:clickToInspect'); + } this.onSelectResultsRecieved(path, true); }); + + this.client.subscribe('track', ({type, eventName, data}: TrackArgs) => { + this.props.logger.track(type, eventName, data); + }); } init() { @@ -563,6 +587,7 @@ export default class Layout extends SonarPlugin { }); if (this.axEnabled()) { + this.props.logger.track('usage', 'accessibility:init'); this.initAX(); } } @@ -626,22 +651,32 @@ export default class Layout extends SonarPlugin { ): Promise> { const {force, ax, forFocusEvent} = options; if (!force) { + const elems = ax ? this.state.AXelements : this.state.elements; + // always force undefined elements and elements that need to be expanded + // over in the main tree (e.g. fragments) ids = ids.filter(id => { return ( - (ax ? this.state.AXelements : this.state.elements)[id] === undefined + !elems[id] || + (elems[id].extraInfo && elems[id].extraInfo.nonAXWithAXChild) ); }); } if (ids.length > 0) { - performance.mark('LayoutInspectorGetNodes'); + // prevents overlapping calls from interfering with each other's logging + const mark = 'LayoutInspectorGetNodes' + this.state.logCounter++; + const eventName = ax + ? 'accessibility:getNodes' + : 'LayoutInspectorGetNodes'; + + performance.mark(mark); return this.client .call(ax ? 'getAXNodes' : 'getNodes', { ids, forFocusEvent, }) .then(({elements}: GetNodesResult) => { - this.props.logger.trackTimeSince('LayoutInspectorGetNodes'); + this.props.logger.trackTimeSince(mark, eventName); return Promise.resolve(elements); }); } else { @@ -670,13 +705,20 @@ export default class Layout extends SonarPlugin { key, type: ax ? 'ExpandAXElement' : 'ExpandElement', }); - performance.mark('LayoutInspectorExpandElement'); + + const mark = ax ? 'ExpandAXElement' : 'LayoutInspectorExpandElement'; + const eventName = ax + ? 'accessibility:expandElement' + : 'LayoutInspectorExpandElement'; + + performance.mark(mark); if (expand) { return this.getChildren(key, ax).then((elements: Array) => { this.dispatchAction({ elements, type: ax ? 'UpdateAXElements' : 'UpdateElements', }); + this.props.logger.trackTimeSince(mark, eventName); // only expand extra components in the main tree when in AX mode if (this.state.inAXMode && !ax) { @@ -687,8 +729,6 @@ export default class Layout extends SonarPlugin { } } } - - this.props.logger.trackTimeSince('LayoutInspectorExpandElement'); return Promise.resolve(elements); }); } else { @@ -731,6 +771,10 @@ export default class Layout extends SonarPlugin { } else { this.expandElement(key, false); } + this.props.logger.track('usage', 'layout:element-expanded', { + id: key, + deep: deep, + }); } if (this.state.AXelements[key]) { @@ -739,12 +783,13 @@ export default class Layout extends SonarPlugin { } else { this.expandElement(key, true); } + if (this.state.inAXMode) { + this.props.logger.track('usage', 'accessibility:elementExpanded', { + id: key, + deep: deep, + }); + } } - - this.props.logger.track('usage', 'layout:element-expanded', { - id: key, - deep: deep, - }); }; onFindClick = () => { @@ -755,8 +800,10 @@ export default class Layout extends SonarPlugin { onToggleAccessibility = () => { const inAXMode = !this.state.inAXMode; + this.props.logger.track('usage', 'accessibility:modeToggled', {inAXMode}); this.dispatchAction({inAXMode, type: 'SetAXMode'}); }; + onToggleAlignment = () => { const isAlignmentMode = !this.state.isAlignmentMode; this.dispatchAction({isAlignmentMode, type: 'SetAlignmentActive'}); @@ -778,11 +825,10 @@ export default class Layout extends SonarPlugin { // element only in AX tree with linked nonAX (litho) element selected } else if ( - (!this.state.elements[selectedKey] || - this.state.elements[selectedKey].name === 'ComponentHost') && - this.state.AXtoNonAXMapping[selectedKey] + !this.state.elements[selectedKey] || + this.state.elements[selectedKey].name === 'ComponentHost' ) { - key = this.state.AXtoNonAXMapping[selectedKey]; + key = this.state.AXtoNonAXMapping[selectedKey] || null; AXkey = selectedKey; // keys are same for both trees or 'linked' element does not exist @@ -802,15 +848,20 @@ export default class Layout extends SonarPlugin { AXkey: AXkey, type: 'SelectElement', }); + this.client.send('setHighlighted', { id: selectedKey, isAlignmentMode: this.state.isAlignmentMode, }); - this.getNodes([key], {force: true, ax: false}).then( - (elements: Array) => { - this.dispatchAction({elements, type: 'UpdateElements'}); - }, - ); + + if (key) { + this.getNodes([key], {force: true, ax: false}).then( + (elements: Array) => { + this.dispatchAction({elements, type: 'UpdateElements'}); + }, + ); + } + if (AXkey) { this.getNodes([AXkey], {force: true, ax: true}).then( (elements: Array) => { @@ -818,6 +869,10 @@ export default class Layout extends SonarPlugin { }, ); } + + if (this.state.inAXMode) { + this.props.logger.track('usage', 'accessibility:selectElement'); + } }); onElementHovered = debounce((key: ?ElementID) => { @@ -852,7 +907,14 @@ export default class Layout extends SonarPlugin { } }); - this.props.logger.track('usage', 'layout:value-changed', {id, value, path}); + const eventName = ax + ? 'accessibility:dataValueChanged' + : 'layout:value-changed'; + this.props.logger.track('usage', eventName, { + id, + value, + path, + }); }; renderSidebar = () => {