From fe9cbbeaa1c3071ecfaa75b725a2ce1f5eb30674 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Sun, 28 Apr 2019 11:54:31 -0700 Subject: [PATCH 1/5] Refactor autofocus and highlight logic To further eliminate the need to pass the parser object to visualization components, the "focus" path is now computed on the fly. That means that every node computes for itself whether it should be open and whether it should be highlighted. This is possible because the tree adapter can answer the question whether a node contains children in cursor range or not. This commit also changes the behavior for nodes that have been opened via focus: If nodes loose focus, they used to stay open. Now, they will close as long as they haven't been explicitly opened. --- website/css/style.css | 2 +- website/src/components/ASTOutput.js | 20 +- website/src/components/getFocusPath.js | 45 -- .../visualization/SelectedNodeContext.js | 32 + website/src/components/visualization/Tree.js | 22 +- .../components/visualization/tree/Element.js | 678 +++++++++++------- .../tree/RecursiveTreeElement.js | 92 --- website/src/containers/ASTOutputContainer.js | 3 +- website/src/core/TreeAdapter.js | 6 +- 9 files changed, 463 insertions(+), 437 deletions(-) delete mode 100644 website/src/components/getFocusPath.js create mode 100644 website/src/components/visualization/SelectedNodeContext.js delete mode 100644 website/src/components/visualization/tree/RecursiveTreeElement.js diff --git a/website/css/style.css b/website/css/style.css index 87e3b9823..0ded3a466 100644 --- a/website/css/style.css +++ b/website/css/style.css @@ -347,7 +347,7 @@ li.entry { } .CodeMirror .marked, -.entry.focused { +.entry.highlighted { border-radius: 2px; background-color: rgba(255,240,6,0.4); } diff --git a/website/src/components/ASTOutput.js b/website/src/components/ASTOutput.js index ab5f478db..a7fa8f421 100644 --- a/website/src/components/ASTOutput.js +++ b/website/src/components/ASTOutput.js @@ -2,9 +2,8 @@ import PropTypes from 'prop-types'; import React from 'react'; import cx from 'classnames'; import visualizations from './visualization'; -import getFocusPath from './getFocusPath'; -const {useState, useMemo} = React; +const {useState} = React; function formatTime(time) { if (!time) { @@ -16,17 +15,9 @@ function formatTime(time) { return `${(time / 1000).toFixed(2)}s`; } -export default function ASTOutput({parser, parseResult={}, cursor=null}) { +export default function ASTOutput({parseResult={}, position=null}) { const [selectedOutput, setSelectedOutput] = useState(0); const {ast=null} = parseResult; - - const focusPath = useMemo( - () => ast && cursor != null ? - getFocusPath(parseResult.ast, cursor, parser) : - [], - [ast, cursor, parser], - ); - let output; if (parseResult.error) { @@ -40,7 +31,7 @@ export default function ASTOutput({parser, parseResult={}, cursor=null}) { { React.createElement( visualizations[selectedOutput], - {parseResult, focusPath} + {parseResult, position} ) } @@ -68,15 +59,14 @@ export default function ASTOutput({parser, parseResult={}, cursor=null}) { {formatTime(parseResult.time)} - {output} + {output} ); } ASTOutput.propTypes = { - parser: PropTypes.object.isRequired, parseResult: PropTypes.object, - cursor: PropTypes.any, + position: PropTypes.number, }; class ErrorBoundary extends React.Component { diff --git a/website/src/components/getFocusPath.js b/website/src/components/getFocusPath.js deleted file mode 100644 index b8d7b9f31..000000000 --- a/website/src/components/getFocusPath.js +++ /dev/null @@ -1,45 +0,0 @@ -function isInRange(range, pos) { - return pos >= range[0] && pos <= range[1]; -} - -export function nodeToRange(parser, node) { - let range = parser.nodeToRange(node); - if (range) { - return range; - } - if (node.length > 0) { - // check first and last child - let rangeFirst = node[0] && parser.nodeToRange(node[0]); - let rangeLast = node[node.length - 1] && - parser.nodeToRange(node[node.length - 1]); - if (rangeFirst && rangeLast) { - return [rangeFirst[0], rangeLast[1]]; - } - } -} - -export default function getFocusPath(node, pos, parser, seen = new Set()) { - seen.add(node); - - let path = []; - let range = nodeToRange(parser, node); - if (range) { - if (isInRange(range, pos)) { - path.push(node); - } else { - return []; - } - } - for (let {value} of parser.forEachProperty(node)) { - if (value && typeof value === 'object' && !seen.has(value)) { - let childPath = getFocusPath(value, pos, parser, seen); - if (childPath.length > 0) { - // if current wasn't added, add it now - childPath = range ? childPath : [node].concat(childPath); - path = path.concat(childPath); - break; - } - } - } - return path; -} diff --git a/website/src/components/visualization/SelectedNodeContext.js b/website/src/components/visualization/SelectedNodeContext.js new file mode 100644 index 000000000..6b680f4ea --- /dev/null +++ b/website/src/components/visualization/SelectedNodeContext.js @@ -0,0 +1,32 @@ +import React from 'react'; + +const SelectedNodeContext = React.createContext(); + +function useSelectedNode() { + const context = React.useContext(SelectedNodeContext); + if (!context) { + throw new Error('useSelectedNode must be used within a SelectedNodeContext'); + } + return context; +} + +let unselectCallback; + +function setSelectedNode(node, cb) { + if (unselectCallback) { + unselectCallback(); + } + if (node) { + global.$node = node; + unselectCallback = cb; + } else { + unselectCallback = null; + delete global.$node; + } +} + +function SelectedNodeProvider(props) { + return ; +} + +export {SelectedNodeProvider, useSelectedNode}; diff --git a/website/src/components/visualization/Tree.js b/website/src/components/visualization/Tree.js index 7b2d27491..d5995cc36 100644 --- a/website/src/components/visualization/Tree.js +++ b/website/src/components/visualization/Tree.js @@ -4,6 +4,7 @@ import React from 'react'; import PubSub from 'pubsub-js'; import {logEvent} from '../../utils/logger'; import {treeAdapterFromParseResult} from '../../core/TreeAdapter.js'; +import {SelectedNodeProvider} from './SelectedNodeContext.js'; import './css/tree.css' @@ -48,7 +49,7 @@ function makeCheckbox(name, settings, updateSettings) { ); } -export default function Tree({focusPath, parseResult}) { +export default function Tree({parseResult, position}) { const [settings, updateSettings] = useReducer(reducer, null, initSettings); const treeAdapter = useMemo( () => treeAdapterFromParseResult(parseResult, settings), @@ -74,20 +75,21 @@ export default function Tree({focusPath, parseResult}) { ))} ); } Tree.propTypes = { - focusPath: PropTypes.array, parseResult: PropTypes.object, - parser: PropTypes.object, + position: PropTypes.number, }; diff --git a/website/src/components/visualization/tree/Element.js b/website/src/components/visualization/tree/Element.js index 7f4451686..354038e1f 100644 --- a/website/src/components/visualization/tree/Element.js +++ b/website/src/components/visualization/tree/Element.js @@ -3,11 +3,21 @@ import CompactObjectView from './CompactObjectView'; import PropTypes from 'prop-types'; import PubSub from 'pubsub-js'; import React from 'react'; -import RecursiveTreeElement from './RecursiveTreeElement'; +import {useSelectedNode} from '../SelectedNodeContext.js'; import cx from 'classnames'; import stringify from '../../../utils/stringify'; +const {useState, useRef, useMemo, useCallback, useEffect} = React; + +function usePrevious(value, initialValue) { + const ref = useRef(initialValue); + useEffect(() => { + ref.current = value; + }); + return ref.current; +} + /* // For debugging function log(f) { @@ -19,323 +29,453 @@ function log(f) { } */ -let lastClickedElement; - -let Element = class extends React.Component { - constructor(props) { - super(props); - this._execFunction = this._execFunction.bind(this); - this._onMouseLeave = this._onMouseLeave.bind(this); - this._onMouseOver = this._onMouseOver.bind(this); - this._toggleClick = this._toggleClick.bind(this); - const {value, name, deepOpen, treeAdapter} = props; - // Some elements should be open by default - let open = - props.open || - props.level === 0 || - deepOpen || - (!!value && treeAdapter.opensByDefault(value, name)); - - this.state = { - open, - deepOpen, - value, - }; - } +const OPEN_STATES = { + DEFAULT: 0, + OPEN: 1, + DEEP_OPEN: 2, + FOCUS_OPEN: 3, + CLOSED: 4, +}; - UNSAFE_componentWillReceiveProps(nextProps) { - this.setState({ - open: nextProps.open || nextProps.deepOpen || this.state.open, - deepOpen: nextProps.deepOpen, - value: nextProps.value, - }); - } +const EVENTS = { + CLICK_SELF: 0, + CLICK_DESCENDANT: 1, + GAIN_FOCUS: 2, + LOOSE_FOCUS: 3, + DEEP_OPEN: 4, +}; - componentWillUnmount() { - if (lastClickedElement === this) { - lastClickedElement = null; - } - } +function transition(currentState, event) { + switch (currentState) { + case OPEN_STATES.DEFAULT: + case OPEN_STATES.CLOSED: + switch (event) { + case EVENTS.DEEP_OPEN: + return OPEN_STATES.DEEP_OPEN; + case EVENTS.GAIN_FOCUS: + return OPEN_STATES.FOCUS_OPEN; + case EVENTS.LOOSE_FOCUS: + return OPEN_STATES.DEFAULT; + } + break; - _shouldAutoFocus(thisProps, nextProps) { - const {focusPath: thisFocusPath} = thisProps; - const {settings: nextSettings, focusPath: nextFocusPath} = nextProps; + case OPEN_STATES.OPEN: + switch (event) { + case EVENTS.DEEP_OPEN: + return OPEN_STATES.DEEP_OPEN; + case EVENTS.GAIN_FOCUS: + case EVENTS.LOOSE_FOCUS: + return currentState; + } + break; - return ( - thisFocusPath !== nextFocusPath && - nextFocusPath.indexOf(nextProps.value) > -1 && - nextSettings.autofocus - ); - } + case OPEN_STATES.DEEP_OPEN: + return OPEN_STATES.DEEP_OPEN; - componentDidMount() { - if (this.props.settings.autofocus) { - this._scrollIntoView(); - } + case OPEN_STATES.FOCUS_OPEN: + switch (event) { + case EVENTS.GAIN_FOCUS: + return OPEN_STATES.FOCUS_OPEN; + case EVENTS.LOOSE_FOCUS: + return OPEN_STATES.DEFAULT; + case EVENTS.DEEP_OPEN: + return OPEN_STATES.DEEP_OPEN; + } + break; } +} + +function useOpenState(openFromParent, isInRange) { + const previousOpenFromParent = usePrevious(openFromParent, false); + const wasInRange = usePrevious(isInRange, false); + const [ownOpenState, setOwnOpenState] = useState(OPEN_STATES.DEFAULT); + const previousOwnOpenState = usePrevious(ownOpenState, OPEN_STATES.DEFAULT); + const previousComputedOpenState = useRef(OPEN_STATES.DEFAULT); + let computedOpenState = previousComputedOpenState.current; - componentDidUpdate(prevProps) { - if (this._shouldAutoFocus(prevProps, this.props)) { - this._scrollIntoView(); + if(ownOpenState !== previousOwnOpenState) { + computedOpenState = ownOpenState; + } else if (wasInRange !== isInRange) { + computedOpenState = transition( + previousComputedOpenState.current, + isInRange && !wasInRange ? EVENTS.GAIN_FOCUS : EVENTS.LOOSE_FOCUS, + ); + if (!isInRange && wasInRange && ownOpenState === OPEN_STATES.CLOSED) { + setOwnOpenState(OPEN_STATES.DEFAULT); } + } else if (openFromParent && !previousOpenFromParent) { + computedOpenState = transition( + previousComputedOpenState.current, + EVENTS.DEEP_OPEN, + ); } - _scrollIntoView() { - const {focusPath, value} = this.props; - if (focusPath.length > 0 && focusPath[focusPath.length -1] === value) { - setTimeout(() => this.container && this.container.scrollIntoView(), 0); + useEffect(() => { + previousComputedOpenState.current = computedOpenState; + }); + + return [ computedOpenState, setOwnOpenState ]; +} + +const Element = React.memo(function Element({ + name, + value, + computed, + open, + level, + treeAdapter, + autofocus, + isInRange, + hasChildrenInRange, + selected, + onClick, + position, +}) { + const opensByDefault = useMemo( + () => treeAdapter.opensByDefault(value, name), + [treeAdapter, value, name], + ); + const [openState, setOpenState] = useOpenState(open, autofocus && isInRange); + const element = useRef(); + + useEffect(() => { + if (autofocus && isInRange && !hasChildrenInRange) { + element.current.scrollIntoView(); } - } + }); - _toggleClick(event) { - const shiftKey = event.shiftKey; - const open = shiftKey || !this.state.open; + const isOpen = openState === OPEN_STATES.DEFAULT ? + opensByDefault : + openState !== OPEN_STATES.CLOSED; - const update = () => { - // Make AST node accessible - if (open) { - global.$node = this.state.value; - } else { - delete global.$node; + const onToggleClick = useCallback( + event => { + const shiftKey = event.shiftKey; + const newOpenState = shiftKey ? OPEN_STATES.DEEP_OPEN : (isOpen ? OPEN_STATES.CLOSED : OPEN_STATES.OPEN); + if (onClick) { + onClick(newOpenState, true); } + setOpenState(newOpenState); + }, + [onClick, isOpen] + ); - this.setState({ - open, - deepOpen: shiftKey, - }); - }; - if (lastClickedElement && lastClickedElement !== this) { - const element = lastClickedElement; - lastClickedElement = open ? this : null; - element.forceUpdate(update); - return; - } else { - lastClickedElement = open ? this : null; - update(); - } - } + const range = treeAdapter.getRange(value); + let onMouseOver; + let onMouseLeave; - _onMouseOver(e) { - e.stopPropagation(); - const {value} = this.state; - PubSub.publish( - 'HIGHLIGHT', - {node: value, range: this.props.treeAdapter.getRange(value)} - ); - } + // enable highlight on hover if node has a range + if (range && level !== 0) { + onMouseOver = event => { + event.stopPropagation(); + PubSub.publish('HIGHLIGHT', {node: value, range}); + }; - _onMouseLeave() { - const {value} = this.state; - PubSub.publish( - 'CLEAR_HIGHLIGHT', - {node: value, range: this.props.treeAdapter.getRange(value)} - ); + onMouseLeave = event => { + event.stopPropagation(); + PubSub.publish('CLEAR_HIGHLIGHT', {node: value, range}); + }; } - _isFocused(level, path, value, open) { - return level !== 0 && - path.indexOf(value) > -1 && - (!open || path[path.length - 1] === value); - } + const clickHandler = useCallback( + () => { + setOpenState(OPEN_STATES.OPEN); + if (onClick) { + onClick(OPEN_STATES.OPEN); + } + }, + [onClick], + ); - _execFunction() { - let state = {error: null}; - try { - state.value = this.state.value.call(this.props.parent); - console.log(state.value); // eslint-disable-line no-console - } catch(err) { - console.error(`Unable to run "${this.props.name}": `, err.message); // eslint-disable-line no-console - state.error = err; + function renderChild(key, value, name, computed) { + if (treeAdapter.isArray(value) || treeAdapter.isObject(value) || typeof value === 'function') { + const ElementType = typeof value === 'function' ? FunctionElement : ElementContainer; + return ( + + ); } - this.setState(state); - } - - _createSubElement(key, value, name, computed) { return ( - ); } - render() { - const { - focusPath, - treeAdapter, - level, - } = this.props; - const { - open, - value, - } = this.state; - const focused = this._isFocused(level, focusPath, value, open); - let valueOutput = null; - let content = null; - let prefix = null; - let suffix = null; - let showToggler = false; - let enableHighlight = false; - - if (value && typeof value === 'object') { - if (!Array.isArray(value)) { - const nodeName = treeAdapter.getNodeName(value); - if (nodeName) { - valueOutput = - - {nodeName}{' '} - {lastClickedElement === this ? - - {' = $node'} - : - null - } - - } - enableHighlight = treeAdapter.getRange(value) && level !== 0; - } else { - enableHighlight = true; + let valueOutput = null; + let content = null; + let prefix = null; + let suffix = null; + let showToggler = false; + + if (value && typeof value === 'object') { + // Render a useful name for object like nodes + if (!treeAdapter.isArray(value)) { + const nodeName = treeAdapter.getNodeName(value); + if (nodeName) { + valueOutput = + + {nodeName}{' '} + {selected ? + + {' = $node'} + : + null + } + } + } - if (typeof value.length === 'number') { - if (value.length > 0 && open) { - prefix = '['; - suffix = ']'; - let elements = [...treeAdapter.walkNode(value)] - .filter(({key}) => key !== 'length') - .map(({key, value, computed}) => this._createSubElement( - key, - value, - Number.isInteger(+key) ? undefined : key, - computed - )); - content =
    {elements}
; - } else { - valueOutput = - - {valueOutput} - - ; - } - showToggler = value.length > 0; + if (typeof value.length === 'number') { + if (value.length > 0 && isOpen) { + prefix = '['; + suffix = ']'; + let elements = Array.from(treeAdapter.walkNode(value)) + .filter(({key}) => key !== 'length') + .map(({key, value, computed}) => renderChild( + key, + value, + Number.isInteger(+key) ? undefined : key, + computed + )); + content =
    {elements}
; } else { - if (open) { - prefix = '{'; - suffix = '}'; - let elements = [...treeAdapter.walkNode(value)] - .map(({key, value, computed}) => this._createSubElement( - key, - value, - key, - computed - )); - content =
    {elements}
; - showToggler = elements.length > 0; - } else { - let keys = [...treeAdapter.walkNode(value)] - .map(({key}) => key); - valueOutput = - - {valueOutput} - - ; - showToggler = keys.length > 0; - } + valueOutput = + + {valueOutput} + + ; } - } else if (typeof value === 'function') { - valueOutput = - - (...) - ; - showToggler = false; + showToggler = value.length > 0; } else { - valueOutput = {stringify(value)}; - showToggler = false; - } - - let name = this.props.name ? - - - {this.props.computed ? - *{this.props.name} : - this.props.name - } - - :  - : - null; - - let classNames = cx({ - entry: true, - focused, - toggable: showToggler, - open, - }); - return ( -
  • this.container = c} - className={classNames} - onMouseOver={enableHighlight ? this._onMouseOver : null} - onMouseLeave={enableHighlight ? this._onMouseLeave : null}> - {name} - - {valueOutput} - - {prefix ?  {prefix} : null} - {content} - {suffix ?
    {suffix}
    : null} - {this.state.error ? + if (isOpen) { + prefix = '{'; + suffix = '}'; + let elements = Array.from(treeAdapter.walkNode(value)) + .map(({key, value, computed}) => renderChild( + key, + value, + key, + computed + )); + content =
      {elements}
    ; + showToggler = elements.length > 0; + } else { + let keys = Array.from(treeAdapter.walkNode(value), ({key}) => key); + valueOutput = - {' '} - - : - null - } -
    • - ); + ; + showToggler = keys.length > 0; + } + } } -}; + + let classNames = cx({ + entry: true, + highlighted: isInRange && (!hasChildrenInRange || !isOpen), + toggable: showToggler, + open: isOpen, + }); + return ( +
  • + {name ? : null} + + {valueOutput} + + {prefix ?  {prefix} : null} + {content} + {suffix ?
    {suffix}
    : null} +
    • + ); +}, +(prevProps, nextProps) => { + return prevProps.name === nextProps.name && + prevProps.value === nextProps.value && + prevProps.computed === nextProps.computed && + prevProps.open === nextProps.open && + prevProps.level === nextProps.level && + prevProps.treeAdapter === nextProps.treeAdapter && + prevProps.autofocus === nextProps.autofocus && + prevProps.selected === nextProps.selected && + prevProps.onClick === nextProps.onClick && + prevProps.isInRange === nextProps.isInRange && + prevProps.hasChildrenInRange === nextProps.hasChildrenInRange && + (!nextProps.isInRange || prevProps.position === nextProps.position); +}); Element.propTypes = { name: PropTypes.string, value: PropTypes.any, computed: PropTypes.bool, open: PropTypes.bool, - deepOpen: PropTypes.bool, - focusPath: PropTypes.array.isRequired, level: PropTypes.number, treeAdapter: PropTypes.object.isRequired, - settings: PropTypes.object.isRequired, + autofocus: PropTypes.bool, parent: PropTypes.oneOfType([ PropTypes.object, PropTypes.array, ]), }; -export default (Element = RecursiveTreeElement(Element)); +const NOT_COMPUTED = {}; + +const FunctionElement = React.memo(function FunctionElement(props) { + const [computedValue, setComputedValue] = useState(NOT_COMPUTED); + const [error, setError] = useState(null); + const {name, value, computed, treeAdapter} = props; + + if (computedValue !== NOT_COMPUTED) { + if (treeAdapter.isArray(computedValue) || treeAdapter.isObject(computedValue)) { + return ( + + ); + } + return ( + + ); + } + + return ( +
  • + {name ? : null} + + { + try { + const computedValue = value.call(parent); + console.log(computedValue); // eslint-disable-line no-console + setComputedValue(computedValue); + } catch(err) { + console.error(`Unable to run "${name}": `, err.message); // eslint-disable-line no-console + setError(err); + } + }}> + (...) + + + {error ? + + {' '} + + : + null + } +
    • + ); +}); + +FunctionElement.propTypes = Element.propTypes; + +const PrimitiveElement = React.memo(function PrimitiveElement({ + name, + value, + computed, +}) { + return ( +
  • + {name ? : null} + + {stringify(value)}; + +
    • + ); +}); + +PrimitiveElement.propTypes = { + name: PropTypes.string, + value: PropTypes.any, + computed: PropTypes.bool, +}; + +const PropertyName = React.memo(function PropertyName({name, computed, onClick}) { + return ( + + + {computed ? *{name} : name } + + :  + + ); +}); + +PropertyName.propTypes = { + name: PropTypes.string, + computed: PropTypes.bool, + onClick: PropTypes.func, +}; + +export default function ElementContainer(props) { + const [selected, setSelected] = useState(false); + const setSelectedNode = useSelectedNode(); + const isInRange = props.treeAdapter.isInRange(props.value, props.position); + const onClick = useCallback( + (state, own) => { + if (own) { + if (state === OPEN_STATES.CLOSED) { + setSelectedNode(null); + setSelected(false); + } else { + setSelectedNode(props.value, () => setSelected(false)); + setSelected(true); + } + } + if (props.onClick) { + props.onClick(state); + } + }, + [props.value, props.onClick], + ); + + return ( + + ); +} + +ElementContainer.propTypes = Element.propTypes; diff --git a/website/src/components/visualization/tree/RecursiveTreeElement.js b/website/src/components/visualization/tree/RecursiveTreeElement.js deleted file mode 100644 index e5f7ebe1b..000000000 --- a/website/src/components/visualization/tree/RecursiveTreeElement.js +++ /dev/null @@ -1,92 +0,0 @@ -import PropTypes from 'prop-types'; -import React from 'react'; - -function shouldAutoFocus({value, settings, focusPath}) { - return !!settings.autofocus && focusPath.indexOf(value) > -1; -} - - -/** - * This is a higher order component the prevents infinite recursion when opening - * the element tree. - */ -export default function RecursiveTreeElement(Element) { - const openValues = new WeakMap(); - - function addValue(value) { - if (openValues.has(value)) { - openValues.set(value, openValues.get(value) + 1); - } else { - openValues.set(value, 1); - } - } - - function removeValue(value) { - let n = openValues.get(value) - 1; - if (n === 0) { - openValues.delete(value); - } else { - openValues.set(value, n); - } - } - - class RecursiveElement extends React.Component { - constructor(props) { - super(props); - let {deepOpen} = props; - let open = shouldAutoFocus(props); - if (props.value && typeof props.value === 'object') { - if (openValues.has(props.value)) { - deepOpen = false; - open = false; - } - addValue(props.value); - } - this.state = {deepOpen, open}; - } - - componentWillUnmount() { - const {value} = this.props; - if (value && typeof value === 'object') { - removeValue(value); - } - } - - UNSAFE_componentWillReceiveProps(props) { - let {deepOpen} = props; - let open = shouldAutoFocus(props); - if (!this.props.value !== props.value) { - if (this.props.value && typeof this.props.value === 'object') { - removeValue(this.props.value); - } - if (props.value && typeof props.value === 'object') { - if (openValues.has(props.value)) { - deepOpen = false; - open = false; - } - addValue(props.value); - } - } - this.setState({deepOpen, open}); - } - - render() { - const {props} = this; - return ( - - ); - } - } - - RecursiveElement.propTypes = { - deepOpen: PropTypes.bool, - value: PropTypes.any, - }; - - return RecursiveElement; - -} diff --git a/website/src/containers/ASTOutputContainer.js b/website/src/containers/ASTOutputContainer.js index 934409fad..3b7d5cf4c 100644 --- a/website/src/containers/ASTOutputContainer.js +++ b/website/src/containers/ASTOutputContainer.js @@ -4,9 +4,8 @@ import * as selectors from '../store/selectors'; function mapStateToProps(state) { return { - parser: selectors.getParser(state), parseResult: selectors.getParseResult(state), - cursor: selectors.getCursor(state), + position: selectors.getCursor(state), }; } diff --git a/website/src/core/TreeAdapter.js b/website/src/core/TreeAdapter.js index 7db1afa15..e2eaaa5c1 100644 --- a/website/src/core/TreeAdapter.js +++ b/website/src/core/TreeAdapter.js @@ -68,12 +68,12 @@ class TreeAdapter { return range[0] <= position && position <= range[1]; } - hasChildrenInRange(node) { - if (!this.isInRange(node)) { + hasChildrenInRange(node, position) { + if (!this.isInRange(node, position)) { return false; } for (const {value: child} of this.walkNode(node)) { - if (this.isInRange(child)) { + if (this.isInRange(child, position)) { return true; } } From bf3fd3b705a4edd2f912a66c268361e94593a067 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Thu, 12 Sep 2019 23:24:39 +0200 Subject: [PATCH 2/5] Fix focus behavior especially for "non-node" elements --- .../components/visualization/tree/Element.js | 12 ++- website/src/core/TreeAdapter.js | 96 ++++++++++--------- website/src/parsers/js/esformatter.js | 32 ++++--- website/src/parsers/js/esprima.js | 18 ++-- website/src/parsers/js/recast.js | 22 +++-- website/src/parsers/js/traceur.js | 32 ++++--- website/src/parsers/js/typescript.js | 40 ++++---- .../parsers/utils/defaultParserInterface.js | 18 ++-- 8 files changed, 147 insertions(+), 123 deletions(-) diff --git a/website/src/components/visualization/tree/Element.js b/website/src/components/visualization/tree/Element.js index 354038e1f..86ee86a60 100644 --- a/website/src/components/visualization/tree/Element.js +++ b/website/src/components/visualization/tree/Element.js @@ -134,8 +134,11 @@ const Element = React.memo(function Element({ const opensByDefault = useMemo( () => treeAdapter.opensByDefault(value, name), [treeAdapter, value, name], + ) || level === 0; + const [openState, setOpenState] = useOpenState( + open, + autofocus && (isInRange || hasChildrenInRange) ); - const [openState, setOpenState] = useOpenState(open, autofocus && isInRange); const element = useRef(); useEffect(() => { @@ -294,7 +297,7 @@ const Element = React.memo(function Element({ let classNames = cx({ entry: true, - highlighted: isInRange && (!hasChildrenInRange || !isOpen), + highlighted: isInRange && (!hasChildrenInRange || !isOpen) || !isInRange && hasChildrenInRange && !isOpen, toggable: showToggler, open: isOpen, }); @@ -326,7 +329,8 @@ const Element = React.memo(function Element({ prevProps.onClick === nextProps.onClick && prevProps.isInRange === nextProps.isInRange && prevProps.hasChildrenInRange === nextProps.hasChildrenInRange && - (!nextProps.isInRange || prevProps.position === nextProps.position); + // + ((nextProps.isInRange || nextProps.hashChildrenInRange) && prevProps.position === nextProps.position); }); Element.propTypes = { @@ -414,7 +418,7 @@ const PrimitiveElement = React.memo(function PrimitiveElement({
  • {name ? : null} - {stringify(value)}; + {stringify(value)}
    • ); diff --git a/website/src/core/TreeAdapter.js b/website/src/core/TreeAdapter.js index e2eaaa5c1..1cd977c6e 100644 --- a/website/src/core/TreeAdapter.js +++ b/website/src/core/TreeAdapter.js @@ -29,38 +29,24 @@ class TreeAdapter { * text and focusing nodes in the tree. */ getRange(node) { - if (!(node && typeof node === 'object')) { + if (node == null) { return null; } - if (this._ranges.has(node)) { return this._ranges.get(node); } const {nodeToRange} = this._adapterOptions; let range = nodeToRange(node); - if (!range) { - // If the node doesn't have location data itself, try to derive it from - // its first and last child. - let first, last; - const iterator = this.walkNode(node); - let next = iterator.next(); - if (!next.done) { - first = last = next.value && next.value.value; - } - while (!(next = iterator.next()).done) { - last = next.value && next.value.value; - } - const rangeFirst = first && nodeToRange(first); - const rangeLast = last && nodeToRange(last); - if (rangeFirst && rangeLast) { - range = [rangeFirst[0], rangeLast[1]]; - } + if (node && typeof node === 'object') { + this._ranges.set(node, range); } - this._ranges.set(node, range); return range; } isInRange(node, position) { + if (!isValidPosition(position)) { + return false; + } const range = this.getRange(node); if (!range) { return false; @@ -68,15 +54,31 @@ class TreeAdapter { return range[0] <= position && position <= range[1]; } - hasChildrenInRange(node, position) { - if (!this.isInRange(node, position)) { + hasChildrenInRange(node, position, seen=new Set()) { + if (!isValidPosition(position)) { return false; } + seen.add(node); + const range = this.getRange(node); + if (range && !this.isInRange(node, position)) { + return false; + } + // Not everything that is rendered has location associated with it (most + // commonly arrays). In such a case we are a looking whether the node + // contains any other nodes with location data (recursively). for (const {value: child} of this.walkNode(node)) { if (this.isInRange(child, position)) { return true; } } + for (const {value: child} of this.walkNode(node)) { + if (seen.has(child)) { + continue; + } + if (this.hasChildrenInRange(child, position, seen)) { + return true; + } + } return false; } @@ -97,33 +99,30 @@ class TreeAdapter { /** * A generator to iterate over each "property" of the node. - * Overwriting _walkNode allows a parser to expose information from a node if - * the node is not implemented as plain JavaScript object. */ *walkNode(node) { - for (const result of this._walkNode(node)) { - if ( - (this._adapterOptions.filters || []).some(filter => { - if (filter.key && !this._filterValues[filter.key]) { - return false; - } - return filter.test(result.value, result.key); - }) - ) { - continue; + if (node != null) { + for (const result of this._adapterOptions.walkNode(node)) { + if ( + (this._adapterOptions.filters || []).some(filter => { + if (filter.key && !this._filterValues[filter.key]) { + return false; + } + return filter.test(result.value, result.key); + }) + ) { + continue; + } + yield result; } - yield result; } } - *_walkNode(node) { - yield* this._adapterOptions.walkNode(node); - } - } const TreeAdapterConfigs = { default: { + filters: [], openByDefault: () => false, nodeToRange: () => null, nodeToName: () => { throw new Error('nodeToName must be passed');}, @@ -149,6 +148,9 @@ const TreeAdapterConfigs = { this.openByDefaultKeys.has(key); }, nodeToRange(node) { + if (!(node && typeof node === 'object')) { + return null; + } if (node.range) { return node.range; } @@ -161,17 +163,23 @@ const TreeAdapterConfigs = { return node.type; }, *walkNode(node) { - for (let prop in node) { - yield { - value: node[prop], - key: prop, - computed: false, + if (node && typeof node === 'object') { + for (let prop in node) { + yield { + value: node[prop], + key: prop, + computed: false, + } } } }, }, }; +function isValidPosition(position) { + return Number.isInteger(position); +} + export function ignoreKeysFilter(keys=new Set(), key, label) { return { key, diff --git a/website/src/parsers/js/esformatter.js b/website/src/parsers/js/esformatter.js index 2a6a4c9fa..c96a7d77b 100644 --- a/website/src/parsers/js/esformatter.js +++ b/website/src/parsers/js/esformatter.js @@ -24,21 +24,23 @@ export default { }, *forEachProperty(node) { - for (let prop in node) { - if (this._ignoredProperties.has(prop)) { - continue; - } - - let value = node[prop]; - - if (node.type !== 'Program' && prop === 'parent') { - value = '[Circular]'; - } - - yield { - value, - key: prop, - computed: false, + if (node && typeof node === 'object') { + for (let prop in node) { + if (this._ignoredProperties.has(prop)) { + continue; + } + + let value = node[prop]; + + if (node.type !== 'Program' && prop === 'parent') { + value = '[Circular]'; + } + + yield { + value, + key: prop, + computed: false, + } } } }, diff --git a/website/src/parsers/js/esprima.js b/website/src/parsers/js/esprima.js index c28522845..6b210b4a8 100644 --- a/website/src/parsers/js/esprima.js +++ b/website/src/parsers/js/esprima.js @@ -21,15 +21,17 @@ export default { }, *forEachProperty(node) { - for (let prop in node) { - if (typeof node[prop] === 'function') { - continue; + if (node && typeof node === 'object') { + for (let prop in node) { + if (typeof node[prop] === 'function') { + continue; + } + yield { + value: node[prop], + key: prop, + computed: false, + }; } - yield { - value: node[prop], - key: prop, - computed: false, - }; } }, diff --git a/website/src/parsers/js/recast.js b/website/src/parsers/js/recast.js index 7e3c3f3d5..52debfcc2 100644 --- a/website/src/parsers/js/recast.js +++ b/website/src/parsers/js/recast.js @@ -79,17 +79,19 @@ export default { _ignoredProperties: new Set(['__clone']), *forEachProperty(node) { - for (let prop in node) { - if ( - this._ignoredProperties.has(prop) || typeof node[prop] === 'function' - ) { - continue; + if (node && typeof node === 'object') { + for (let prop in node) { + if ( + this._ignoredProperties.has(prop) || typeof node[prop] === 'function' + ) { + continue; + } + yield { + value: node[prop], + key: prop, + computed: false, + }; } - yield { - value: node[prop], - key: prop, - computed: false, - }; } }, diff --git a/website/src/parsers/js/traceur.js b/website/src/parsers/js/traceur.js index cfd193818..32c74a701 100644 --- a/website/src/parsers/js/traceur.js +++ b/website/src/parsers/js/traceur.js @@ -62,22 +62,24 @@ export default { }, *forEachProperty(node) { - if ('type' in node) { - yield { - value: node.type, - key: 'type', + if (node && typeof node === 'object') { + if ('type' in node) { + yield { + value: node.type, + key: 'type', + } } - } - for (let prop in node) { - if (prop === 'line_' || prop === 'column_') { - prop = prop.slice(0, -1); - } - if (prop === 'type' || prop === 'lineNumberTable') { - continue; - } - yield { - value: node[prop], - key: prop, + for (let prop in node) { + if (prop === 'line_' || prop === 'column_') { + prop = prop.slice(0, -1); + } + if (prop === 'type' || prop === 'lineNumberTable') { + continue; + } + yield { + value: node[prop], + key: prop, + } } } }, diff --git a/website/src/parsers/js/typescript.js b/website/src/parsers/js/typescript.js index 462b165c6..766f1b9e6 100644 --- a/website/src/parsers/js/typescript.js +++ b/website/src/parsers/js/typescript.js @@ -94,26 +94,28 @@ export default { ]), *forEachProperty(node) { - for (let prop in node) { - if (this._ignoredProperties.has(prop) || prop.charAt(0) === '_') { - continue; + if (node && typeof node === 'object') { + for (let prop in node) { + if (this._ignoredProperties.has(prop) || prop.charAt(0) === '_') { + continue; + } + yield { + value: node[prop], + key: prop, + }; + } + if (node.parent) { + yield { + value: getComments(node), + key: 'leadingComments', + computed: true, + }; + yield { + value: getComments(node, true), + key: 'trailingComments', + computed: true, + }; } - yield { - value: node[prop], - key: prop, - }; - } - if (node.parent) { - yield { - value: getComments(node), - key: 'leadingComments', - computed: true, - }; - yield { - value: getComments(node, true), - key: 'trailingComments', - computed: true, - }; } }, diff --git a/website/src/parsers/utils/defaultParserInterface.js b/website/src/parsers/utils/defaultParserInterface.js index ef65255c7..e0c34ce52 100644 --- a/website/src/parsers/utils/defaultParserInterface.js +++ b/website/src/parsers/utils/defaultParserInterface.js @@ -85,14 +85,16 @@ export default { * is not implemnted as plain JavaScript object. */ *forEachProperty(node) { - for (let prop in node) { - if (this._ignoredProperties.has(prop)) { - continue; - } - yield { - value: node[prop], - key: prop, - computed: false, + if (node && typeof node === 'object') { + for (let prop in node) { + if (this._ignoredProperties.has(prop)) { + continue; + } + yield { + value: node[prop], + key: prop, + computed: false, + } } } }, From 848baf202f31ff2b93103b632888e20093cf42e8 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Fri, 13 Sep 2019 00:10:35 +0200 Subject: [PATCH 3/5] Fix scroll into view with multiple items See the comment in `focusNodes` for more information. --- website/src/components/visualization/Tree.js | 11 +++- .../components/visualization/focusNodes.js | 65 +++++++++++++++++++ .../components/visualization/tree/Element.js | 10 ++- 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 website/src/components/visualization/focusNodes.js diff --git a/website/src/components/visualization/Tree.js b/website/src/components/visualization/Tree.js index d5995cc36..57b200a99 100644 --- a/website/src/components/visualization/Tree.js +++ b/website/src/components/visualization/Tree.js @@ -5,10 +5,11 @@ import PubSub from 'pubsub-js'; import {logEvent} from '../../utils/logger'; import {treeAdapterFromParseResult} from '../../core/TreeAdapter.js'; import {SelectedNodeProvider} from './SelectedNodeContext.js'; +import focusNodes from './focusNodes.js' import './css/tree.css' -const {useReducer, useMemo} = React; +const {useReducer, useMemo, useRef, useLayoutEffect} = React; const STORAGE_KEY = 'tree_settings'; @@ -55,6 +56,12 @@ export default function Tree({parseResult, position}) { () => treeAdapterFromParseResult(parseResult, settings), [parseResult.treeAdapter, settings], ); + const rootElement = useRef(); + + focusNodes('init'); + useLayoutEffect(() => { + focusNodes('focus', rootElement); + }); return (
    @@ -74,7 +81,7 @@ export default function Tree({parseResult, position}) { ))}
    -
      {PubSub.publish('CLEAR_HIGHLIGHT');}}> +
        {PubSub.publish('CLEAR_HIGHLIGHT');}}> 1) { + const rootRect = root.getBoundingClientRect(); + const center = (rootRect.y + rootRect.height) / 2 + rootRect.y; + const closest = Array.from(nodes).reduce((closest, element) => { + if (!element.current) { + return closest; + } + const elementRect = element.current.getBoundingClientRect(); + const distance = elementRect.y - center; + const minDistance = Math.min( + Math.abs(distance), + Math.abs(distance + elementRect.height) + ); + + if (!closest || closest[1] > minDistance) { + return [element.current, minDistance]; + } + return closest; + }, null); + if (closest) { + closest[0].scrollIntoView(); + } + } + } catch (e) { + // eslint-disable-next-line no-console + console.error('Unable to scroll node into view:', e.message); + } + + } + } +} diff --git a/website/src/components/visualization/tree/Element.js b/website/src/components/visualization/tree/Element.js index 86ee86a60..e446c5b99 100644 --- a/website/src/components/visualization/tree/Element.js +++ b/website/src/components/visualization/tree/Element.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import PubSub from 'pubsub-js'; import React from 'react'; import {useSelectedNode} from '../SelectedNodeContext.js'; +import focusNodes from '../focusNodes.js' import cx from 'classnames'; import stringify from '../../../utils/stringify'; @@ -140,12 +141,9 @@ const Element = React.memo(function Element({ autofocus && (isInRange || hasChildrenInRange) ); const element = useRef(); - - useEffect(() => { - if (autofocus && isInRange && !hasChildrenInRange) { - element.current.scrollIntoView(); - } - }); + if (autofocus && isInRange && !hasChildrenInRange) { + focusNodes('add', element); + } const isOpen = openState === OPEN_STATES.DEFAULT ? opensByDefault : From 2b877a8924e09e2f8cf9044459f4685b71e0a975 Mon Sep 17 00:00:00 2001 From: Adam Pavlisin Date: Wed, 9 Oct 2024 16:32:15 +0200 Subject: [PATCH 4/5] Create callstack-reviewer.yml --- .../workflows/callstack-reviewer.yml | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/workflows/callstack-reviewer.yml diff --git a/.github/ISSUE_TEMPLATE/workflows/callstack-reviewer.yml b/.github/ISSUE_TEMPLATE/workflows/callstack-reviewer.yml new file mode 100644 index 000000000..1bca86841 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/workflows/callstack-reviewer.yml @@ -0,0 +1,31 @@ +name: Callstack.ai PR Review + +on: + workflow_dispatch: + inputs: + config: + type: string + description: "config for reviewer" + required: true + head: + type: string + description: "head commit sha" + required: true + base: + type: string + description: "base commit sha" + required: false + +jobs: + callstack_pr_review_job: + runs-on: ubuntu-latest + steps: + - name: Review PR + uses: callstackai/action@v1.0.7 + with: + config: ${{ inputs.config }} + head: ${{ inputs.head }} + openai_key: ${{ secrets.OPENAI_KEY_2 }} + export: /code/chats.json + channel: pre-release + tag: develop From b97dbf986de857b6a56888ec1ceecaa42e38f695 Mon Sep 17 00:00:00 2001 From: Adam Pavlisin Date: Wed, 9 Oct 2024 16:33:58 +0200 Subject: [PATCH 5/5] Create callstack-reviewer.yml --- .github/workflows/callstack-reviewer.yml | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/callstack-reviewer.yml diff --git a/.github/workflows/callstack-reviewer.yml b/.github/workflows/callstack-reviewer.yml new file mode 100644 index 000000000..1bca86841 --- /dev/null +++ b/.github/workflows/callstack-reviewer.yml @@ -0,0 +1,31 @@ +name: Callstack.ai PR Review + +on: + workflow_dispatch: + inputs: + config: + type: string + description: "config for reviewer" + required: true + head: + type: string + description: "head commit sha" + required: true + base: + type: string + description: "base commit sha" + required: false + +jobs: + callstack_pr_review_job: + runs-on: ubuntu-latest + steps: + - name: Review PR + uses: callstackai/action@v1.0.7 + with: + config: ${{ inputs.config }} + head: ${{ inputs.head }} + openai_key: ${{ secrets.OPENAI_KEY_2 }} + export: /code/chats.json + channel: pre-release + tag: develop