Improve multiple element selector UI
Summary: Layer selection is pretty easy to miss, as reported in for example: https://fb.workplace.com/groups/flippersupport/permalink/1098169193997071/ Moved the layer selection to the top of the view and gave it some highlighting + dynamic height. The section is no longer collapsible. Changelog: [Layout] Make the layer selection more prominent Reviewed By: priteshrnandgaonkar Differential Revision: D27708650 fbshipit-source-id: c86a55c3a20794aee86e64b6766b2ca4dd6b563f
This commit is contained in:
committed by
Facebook GitHub Bot
parent
5db2ef1275
commit
0fe879c838
@@ -10,6 +10,7 @@
|
||||
import {Component} from 'react';
|
||||
import {Elements, DecorateRow, ContextMenuExtension} from './elements';
|
||||
import React from 'react';
|
||||
import {Layout} from '../Layout';
|
||||
|
||||
export type ElementID = string;
|
||||
|
||||
@@ -72,11 +73,16 @@ export type ElementsInspectorProps = {
|
||||
alternateRowColor?: boolean;
|
||||
contextMenuExtensions?: () => Array<ContextMenuExtension>;
|
||||
decorateRow?: DecorateRow;
|
||||
/**
|
||||
* By default the ElementsInspector takes all available space and is scrollab.e
|
||||
*/
|
||||
scrollable?: boolean;
|
||||
};
|
||||
|
||||
export class ElementsInspector extends Component<ElementsInspectorProps> {
|
||||
static defaultProps = {
|
||||
alternateRowColor: true,
|
||||
scrollable: true,
|
||||
};
|
||||
render() {
|
||||
const {
|
||||
@@ -91,9 +97,10 @@ export class ElementsInspector extends Component<ElementsInspectorProps> {
|
||||
alternateRowColor,
|
||||
contextMenuExtensions,
|
||||
decorateRow,
|
||||
scrollable,
|
||||
} = this.props;
|
||||
|
||||
return (
|
||||
const renderedElems = (
|
||||
<Elements
|
||||
onElementExpanded={onElementExpanded}
|
||||
onElementSelected={onElementSelected}
|
||||
@@ -108,5 +115,10 @@ export class ElementsInspector extends Component<ElementsInspectorProps> {
|
||||
decorateRow={decorateRow}
|
||||
/>
|
||||
);
|
||||
if (scrollable) {
|
||||
return <Layout.ScrollContainer>{renderedElems}</Layout.ScrollContainer>;
|
||||
} else {
|
||||
return renderedElems;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -449,8 +449,6 @@ function containsKeyInSearchResults(
|
||||
const ElementsContainer = styled('div')({
|
||||
display: 'table',
|
||||
backgroundColor: theme.backgroundDefault,
|
||||
minHeight: '100%',
|
||||
minWidth: '100%',
|
||||
});
|
||||
ElementsContainer.displayName = 'Elements:ElementsContainer';
|
||||
|
||||
@@ -489,7 +487,6 @@ export class Elements extends PureComponent<ElementsProps, ElementsState> {
|
||||
static defaultProps = {
|
||||
alternateRowColor: true,
|
||||
};
|
||||
_outerRef = React.createRef<HTMLDivElement>();
|
||||
constructor(props: ElementsProps, context: Object) {
|
||||
super(props, context);
|
||||
this.state = {
|
||||
@@ -690,24 +687,14 @@ export class Elements extends PureComponent<ElementsProps, ElementsState> {
|
||||
};
|
||||
|
||||
scrollToSelectionRefHandler = (selectedRow: HTMLDivElement | null) => {
|
||||
if (
|
||||
!selectedRow ||
|
||||
!this._outerRef.current ||
|
||||
this.props.selected === this.state.scrolledElement
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this.setState({scrolledElement: this.props.selected});
|
||||
const outer = this._outerRef.current;
|
||||
if (outer.scrollTo) {
|
||||
outer.scrollTo({
|
||||
top: this._calculateScrollTop(
|
||||
outer.offsetHeight,
|
||||
outer.scrollTop,
|
||||
selectedRow.offsetHeight,
|
||||
selectedRow.offsetTop,
|
||||
),
|
||||
if (selectedRow && this.state.scrolledElement !== this.props.selected) {
|
||||
// second child is the element containing the element name
|
||||
// by scrolling to the second element, we make sure padding is addressed and we scroll horizontally as well
|
||||
selectedRow?.children[1]?.scrollIntoView?.({
|
||||
block: 'center',
|
||||
inline: 'center',
|
||||
});
|
||||
this.setState({scrolledElement: this.props.selected});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -761,7 +748,7 @@ export class Elements extends PureComponent<ElementsProps, ElementsState> {
|
||||
contextMenuExtensions={contextMenuExtensions}
|
||||
decorateRow={decorateRow}
|
||||
forwardedRef={
|
||||
selected == row.key && this.state.scrolledElement !== selected
|
||||
selected == row.key // && this.state.scrolledElement !== selected
|
||||
? this.scrollToSelectionRefHandler
|
||||
: null
|
||||
}
|
||||
@@ -771,11 +758,9 @@ export class Elements extends PureComponent<ElementsProps, ElementsState> {
|
||||
|
||||
render() {
|
||||
return (
|
||||
<Layout.ScrollContainer ref={this._outerRef}>
|
||||
<ElementsContainer onKeyDown={this.onKeyDown} tabIndex={0}>
|
||||
{this.state.flatElements.map(this.buildRow)}
|
||||
</ElementsContainer>
|
||||
</Layout.ScrollContainer>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,19 +13,13 @@ import {
|
||||
PluginClient,
|
||||
ElementsInspector,
|
||||
ElementSearchResultSet,
|
||||
FlexColumn,
|
||||
styled,
|
||||
} from 'flipper';
|
||||
import {debounce} from 'lodash';
|
||||
import {Component} from 'react';
|
||||
import {PersistedState, ElementMap} from './';
|
||||
import React from 'react';
|
||||
import MultipleSelectorSection from './MultipleSelectionSection';
|
||||
|
||||
const ElementsInspectorContainer = styled(FlexColumn)({
|
||||
width: '100%',
|
||||
justifyContent: 'space-between',
|
||||
});
|
||||
import {Layout} from 'flipper-plugin';
|
||||
|
||||
type GetNodesOptions = {
|
||||
force?: boolean;
|
||||
@@ -448,7 +442,17 @@ export default class Inspector extends Component<Props, State> {
|
||||
: this.state.elementSelector;
|
||||
|
||||
return this.root() ? (
|
||||
<ElementsInspectorContainer>
|
||||
<Layout.Top>
|
||||
{selectorData && selectorData.leaves.length > 1 ? (
|
||||
<MultipleSelectorSection
|
||||
initialSelectedElement={this.selected()}
|
||||
elements={selectorData.elements}
|
||||
onElementSelected={this.onElementSelectedAndExpanded}
|
||||
onElementHovered={this.onElementHovered}
|
||||
/>
|
||||
) : (
|
||||
<div />
|
||||
)}
|
||||
<ElementsInspector
|
||||
onElementSelected={this.onElementSelectedAtMainSection}
|
||||
onElementHovered={this.onElementHovered}
|
||||
@@ -460,15 +464,7 @@ export default class Inspector extends Component<Props, State> {
|
||||
focused={this.focused()}
|
||||
contextMenuExtensions={this.getAXContextMenuExtensions}
|
||||
/>
|
||||
{selectorData && selectorData.leaves.length > 1 ? (
|
||||
<MultipleSelectorSection
|
||||
initialSelectedElement={this.selected()}
|
||||
elements={selectorData.elements}
|
||||
onElementSelected={this.onElementSelectedAndExpanded}
|
||||
onElementHovered={this.onElementHovered}
|
||||
/>
|
||||
) : null}
|
||||
</ElementsInspectorContainer>
|
||||
</Layout.Top>
|
||||
) : null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,41 +7,16 @@
|
||||
* @format
|
||||
*/
|
||||
|
||||
import {
|
||||
FlexColumn,
|
||||
FlexBox,
|
||||
Element,
|
||||
ElementID,
|
||||
ElementsInspector,
|
||||
Glyph,
|
||||
colors,
|
||||
styled,
|
||||
} from 'flipper';
|
||||
import {Typography} from 'antd';
|
||||
import {Element, ElementID, ElementsInspector, styled} from 'flipper';
|
||||
import {Layout, theme} from 'flipper-plugin';
|
||||
import React, {memo, useState} from 'react';
|
||||
|
||||
const MultipleSelectorSectionContainer = styled(FlexColumn)({
|
||||
maxHeight: 100,
|
||||
});
|
||||
|
||||
const MultipleSelectorSectionTitle = styled(FlexBox)({
|
||||
cursor: 'pointer',
|
||||
backgroundColor: '#f6f7f9',
|
||||
padding: '2px',
|
||||
paddingLeft: '9px',
|
||||
width: '325px',
|
||||
height: '20px',
|
||||
fontWeight: 500,
|
||||
boxShadow: '2px 2px 2px #ccc',
|
||||
border: `1px solid ${colors.light20}`,
|
||||
borderTopLeftRadius: '4px',
|
||||
borderTopRightRadius: '4px',
|
||||
textAlign: 'center',
|
||||
});
|
||||
|
||||
const Chevron = styled(Glyph)({
|
||||
marginRight: 4,
|
||||
marginLeft: -2,
|
||||
marginBottom: 1,
|
||||
const MultipleSelectorSectionContainer = styled(Layout.Container)({
|
||||
padding: theme.space.medium,
|
||||
background: theme.backgroundWash,
|
||||
borderTop: `${theme.space.tiny}px solid ${theme.warningColor}`,
|
||||
borderBottom: `${theme.space.tiny}px solid ${theme.warningColor}`,
|
||||
});
|
||||
|
||||
type MultipleSelectorSectionProps = {
|
||||
@@ -65,17 +40,12 @@ const MultipleSelectorSection: React.FC<MultipleSelectorSectionProps> = memo(
|
||||
const [selectedId, setSelectedId] = useState<ElementID | null | undefined>(
|
||||
initialSelectedElement,
|
||||
);
|
||||
const [collapsed, setCollapsed] = useState(false);
|
||||
return (
|
||||
<MultipleSelectorSectionContainer>
|
||||
<MultipleSelectorSectionTitle
|
||||
onClick={() => {
|
||||
setCollapsed(!collapsed);
|
||||
}}>
|
||||
<Chevron name={collapsed ? 'chevron-up' : 'chevron-down'} size={12} />
|
||||
Multiple elements found at the target coordinates
|
||||
</MultipleSelectorSectionTitle>
|
||||
{!collapsed && (
|
||||
<MultipleSelectorSectionContainer gap>
|
||||
<Typography.Text>
|
||||
Multiple elements were found at the target coordinates. Please select
|
||||
one:
|
||||
</Typography.Text>
|
||||
<ElementsInspector
|
||||
onElementSelected={(key: string) => {
|
||||
setSelectedId(key);
|
||||
@@ -86,8 +56,8 @@ const MultipleSelectorSection: React.FC<MultipleSelectorSectionProps> = memo(
|
||||
root={null}
|
||||
selected={selectedId}
|
||||
elements={elements}
|
||||
scrollable={false}
|
||||
/>
|
||||
)}
|
||||
</MultipleSelectorSectionContainer>
|
||||
);
|
||||
},
|
||||
|
||||
@@ -40,30 +40,6 @@ test('rendering a single element', () => {
|
||||
expect(res.queryAllByText(name).length).toBe(1);
|
||||
});
|
||||
|
||||
test('collapsing an element', () => {
|
||||
const id = 'id1';
|
||||
const name = 'id_name';
|
||||
const element: Element = {...dummyElmentData, id, name};
|
||||
const res = render(
|
||||
<MultipleSelectorSection
|
||||
initialSelectedElement={null}
|
||||
elements={{[id]: element}}
|
||||
onElementSelected={() => {}}
|
||||
onElementHovered={null}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(res.queryAllByText(name).length).toBe(1);
|
||||
|
||||
// collapse the view
|
||||
fireEvent.click(res.getByText(TITLE_STRING));
|
||||
expect(res.queryAllByText(name).length).toBe(0);
|
||||
|
||||
// re-expand the view
|
||||
fireEvent.click(res.getByText(TITLE_STRING));
|
||||
expect(res.queryAllByText(name).length).toBe(1);
|
||||
});
|
||||
|
||||
test('clicking on elements', () => {
|
||||
const ids = ['id1', 'id2', 'id3'];
|
||||
const names = ['id_name_first', 'id_name_second', 'id_name_third'];
|
||||
|
||||
Reference in New Issue
Block a user