Skip to content

Commit 1b18f55

Browse files
priyajeetpriyajeet
authored
Fix: Fix autoFocus issue and add shortcut for breadcrumbs (#22)
Fix: Remove redundant parent null check when renaming files
1 parent 2c2f7ed commit 1b18f55

9 files changed

Lines changed: 106 additions & 48 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ When the item list is focused, either manually by clicking on it or programatica
9494
| Shift + D | Download an item |
9595
| g then f | Navigates to the root folder |
9696
| g then u | Upload to the current folder |
97+
| g then b | Focuses the root breadcrumb |
9798

9899

99100

@@ -159,6 +160,9 @@ When the item list is focused, either manually by clicking on it or programatica
159160
| g then s | Show selected items |
160161
| g then c | Choose |
161162
| g then x | Cancel |
163+
| g then b | Focuses the root breadcrumb |
164+
165+
162166

163167
## Content Uploader ([Documentation](https://developer.box.com/docs/box-content-uploader))
164168
<img src="https://user-images.githubusercontent.com/1075325/27887153-09243762-6194-11e7-8d2d-cf654d9364bc.png" width="75%"/>
@@ -197,6 +201,7 @@ render(
197201
| sharedLinkPassword | string | | *See the [developer docs](https://developer.box.com/docs/box-content-uploader#section-options).* |
198202

199203

204+
200205
## Content Tree ([Documentation](https://developer.box.com/docs/box-content-tree))
201206

202207
<img src="https://user-images.githubusercontent.com/1075325/27887155-092e7362-6194-11e7-877d-157726789bef.png" width="75%"/>
@@ -236,6 +241,7 @@ render(
236241
| sharedLinkPassword | string | | *See the [developer docs](https://developer.box.com/docs/box-content-tree#section-options).* |
237242

238243

244+
239245
## Content Preview ([Documentation](https://developer.box.com/docs/box-content-preview))
240246

241247
<img src="https://user-images.githubusercontent.com/1075325/27419184-596b485c-56d4-11e7-8d42-c65328089c95.png" width="75%"/>

‎src/components/ContentExplorer/ContentExplorer.js‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ type State = {
100100
isPreviewModalOpen: boolean,
101101
isLoading: boolean,
102102
errorCode: string,
103-
focusedRow: number
103+
focusedRow: number,
104+
firstLoad: boolean
104105
};
105106

106107
type DefaultProps = {|
@@ -205,7 +206,8 @@ class ContentExplorer extends Component<DefaultProps, Props, State> {
205206
isPreviewModalOpen: false,
206207
isLoading: false,
207208
errorCode: '',
208-
focusedRow: 0
209+
focusedRow: 0,
210+
firstLoad: true
209211
};
210212
}
211213

@@ -316,12 +318,18 @@ class ContentExplorer extends Component<DefaultProps, Props, State> {
316318
*/
317319
finishNavigation() {
318320
const { onNavigate, autoFocus }: Props = this.props;
319-
const { currentCollection: { percentLoaded, boxItem } }: State = this.state;
321+
const { firstLoad, currentCollection: { percentLoaded, boxItem } }: State = this.state;
320322
onNavigate(cloneDeep(boxItem));
321323

322324
// Don't focus the grid until its loaded and user is not already on an interactable element
323-
if (autoFocus && percentLoaded === 100 && !isFocusableElement(document.activeElement)) {
325+
if (percentLoaded === 100 && !isFocusableElement(document.activeElement)) {
326+
// If loading for the very first time, only focus if autoFocus is true
327+
if (firstLoad && !autoFocus) {
328+
this.setState({ firstLoad: false });
329+
return;
330+
}
324331
focus(this.rootElement, '.bce-item-row');
332+
this.setState({ focusedRow: 0 });
325333
}
326334
}
327335

@@ -751,8 +759,8 @@ class ContentExplorer extends Component<DefaultProps, Props, State> {
751759
return;
752760
}
753761

754-
const { id, permissions, parent, type }: BoxItem = selected;
755-
if (!id || !permissions || !parent || !type) {
762+
const { id, permissions, type }: BoxItem = selected;
763+
if (!id || !permissions || !type) {
756764
return;
757765
}
758766

@@ -940,8 +948,17 @@ class ContentExplorer extends Component<DefaultProps, Props, State> {
940948
case '/':
941949
focus(this.rootElement, '.buik-search input[type="search"]', false);
942950
break;
951+
case 'arrowdown':
952+
focus(this.rootElement, '.bce-item-row', false);
953+
this.setState({ focusedRow: 0 });
954+
break;
943955
case 'g':
944956
break;
957+
case 'b':
958+
if (this.globalModifier) {
959+
focus(this.rootElement, '.buik-breadcrumb button', false);
960+
}
961+
break;
945962
case 'f':
946963
if (this.globalModifier) {
947964
this.fetchFolder(rootFolderId);

‎src/components/ContentExplorer/ItemList.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ const ItemList = ({
128128
scrollToRow={focusedRow}
129129
onScrollToChange={({ scrollToRow }) => focus(rootElement, `.bce-item-row-${scrollToRow}`)}
130130
>
131-
{({ onSectionRendered, scrollToRow }) =>
131+
{({ onSectionRendered, scrollToRow, focusOnRender }) =>
132132
<AutoSizer>
133133
{({ width, height }) =>
134134
<Table
@@ -147,7 +147,9 @@ const ItemList = ({
147147
scrollToIndex={scrollToRow}
148148
onRowsRendered={({ startIndex, stopIndex }) => {
149149
onSectionRendered({ rowStartIndex: startIndex, rowStopIndex: stopIndex });
150-
focus(rootElement, `.bce-item-row-${scrollToRow}`);
150+
if (focusOnRender) {
151+
focus(rootElement, `.bce-item-row-${scrollToRow}`);
152+
}
151153
}}
152154
>
153155
<Column

‎src/components/ContentPicker/Content.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ type Props = {
2222
canSetShareAccess: boolean,
2323
onItemClick: Function,
2424
onItemSelect: Function,
25+
onFocusChange: Function,
2526
onShareAccessChange: Function,
2627
extensionsWhitelist: string[],
2728
hasHitSelectionLimit: boolean,
@@ -56,6 +57,7 @@ const Content = ({
5657
onItemClick,
5758
onItemSelect,
5859
onShareAccessChange,
60+
onFocusChange,
5961
extensionsWhitelist,
6062
getLocalizedMessage
6163
}: Props) =>
@@ -82,6 +84,7 @@ const Content = ({
8284
selectableType={selectableType}
8385
onItemSelect={onItemSelect}
8486
onItemClick={onItemClick}
87+
onFocusChange={onFocusChange}
8588
onShareAccessChange={onShareAccessChange}
8689
extensionsWhitelist={extensionsWhitelist}
8790
getLocalizedMessage={getLocalizedMessage}

‎src/components/ContentPicker/ContentPicker.js‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ type State = {
8585
searchQuery: string,
8686
view: View,
8787
isUploadModalOpen: boolean,
88-
focusedRow: number
88+
focusedRow: number,
89+
firstLoad: boolean
8990
};
9091

9192
type DefaultProps = {|
@@ -174,7 +175,8 @@ class ContentPicker extends Component<DefaultProps, Props, State> {
174175
searchQuery: '',
175176
view: VIEW_FOLDER,
176177
isUploadModalOpen: false,
177-
focusedRow: 0
178+
focusedRow: 0,
179+
firstLoad: true
178180
};
179181
}
180182

@@ -329,11 +331,17 @@ class ContentPicker extends Component<DefaultProps, Props, State> {
329331
*/
330332
finishNavigation() {
331333
const { autoFocus }: Props = this.props;
332-
const { currentCollection: { percentLoaded } }: State = this.state;
334+
const { firstLoad, currentCollection: { percentLoaded } }: State = this.state;
333335

334336
// Don't focus the grid until its loaded and user is not already on an interactable element
335-
if (autoFocus && percentLoaded === 100 && !isFocusableElement(document.activeElement)) {
337+
if (percentLoaded === 100 && !isFocusableElement(document.activeElement)) {
338+
// If loading for the very first time, only focus if autoFocus is true
339+
if (firstLoad && !autoFocus) {
340+
this.setState({ firstLoad: false });
341+
return;
342+
}
336343
focus(this.rootElement, '.bcp-item-row');
344+
this.setState({ focusedRow: 0 });
337345
}
338346
}
339347

@@ -671,8 +679,17 @@ class ContentPicker extends Component<DefaultProps, Props, State> {
671679
case '/':
672680
focus(this.rootElement, '.buik-search input[type="search"]', false);
673681
break;
682+
case 'arrowdown':
683+
focus(this.rootElement, '.bcp-item-row', false);
684+
this.setState({ focusedRow: 0 });
685+
break;
674686
case 'g':
675687
break;
688+
case 'b':
689+
if (this.globalModifier) {
690+
focus(this.rootElement, '.buik-breadcrumb button', false);
691+
}
692+
break;
676693
case 'f':
677694
if (this.globalModifier) {
678695
this.fetchFolder(rootFolderId);
@@ -707,6 +724,17 @@ class ContentPicker extends Component<DefaultProps, Props, State> {
707724
event.preventDefault();
708725
};
709726

727+
/**
728+
* Updates the focused row based on key binder
729+
*
730+
* @private
731+
* @param {number} focusedRow - the row index thats focused
732+
* @return {void}
733+
*/
734+
onFocusChange = (focusedRow: number) => {
735+
this.setState({ focusedRow });
736+
};
737+
710738
/**
711739
* Renders the file picker
712740
*
@@ -787,6 +815,7 @@ class ContentPicker extends Component<DefaultProps, Props, State> {
787815
tableRef={this.tableRef}
788816
onItemSelect={this.select}
789817
onItemClick={this.onItemClick}
818+
onFocusChange={this.onFocusChange}
790819
onShareAccessChange={this.changeShareAccess}
791820
getLocalizedMessage={getLocalizedMessage}
792821
/>

‎src/components/ContentPicker/ItemList.js‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type Props = {
3131
selectableType: string,
3232
hasHitSelectionLimit: boolean,
3333
onShareAccessChange: Function,
34+
onFocusChange: Function,
3435
extensionsWhitelist: string[],
3536
getLocalizedMessage: Function,
3637
currentCollection: Collection,
@@ -51,6 +52,7 @@ const ItemList = ({
5152
onItemSelect,
5253
onItemClick,
5354
onShareAccessChange,
55+
onFocusChange,
5456
currentCollection,
5557
tableRef,
5658
getLocalizedMessage
@@ -81,13 +83,23 @@ const ItemList = ({
8183
});
8284
};
8385

84-
const onRowClick = ({ event, rowData }: { event: Event & { target: HTMLElement }, rowData: BoxItem }) => {
86+
const onRowClick = ({
87+
event,
88+
rowData,
89+
index
90+
}: {
91+
event: Event & { target: HTMLElement },
92+
rowData: BoxItem,
93+
index: number
94+
}) => {
8595
// If the click is happening on a clickable element on the item row, ignore row selection
8696
if (
8797
isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData) &&
8898
!isFocusableElement(event.target)
8999
) {
90100
onItemSelect(rowData);
101+
} else {
102+
onFocusChange(index);
91103
}
92104
};
93105

@@ -102,7 +114,7 @@ const ItemList = ({
102114
scrollToRow={focusedRow}
103115
onScrollToChange={({ scrollToRow }) => focus(rootElement, `.bcp-item-row-${scrollToRow}`)}
104116
>
105-
{({ onSectionRendered, scrollToRow }) =>
117+
{({ onSectionRendered, scrollToRow, focusOnRender }) =>
106118
<AutoSizer>
107119
{({ width, height }) =>
108120
<Table
@@ -119,7 +131,9 @@ const ItemList = ({
119131
scrollToIndex={scrollToRow}
120132
onRowsRendered={({ startIndex, stopIndex }) => {
121133
onSectionRendered({ rowStartIndex: startIndex, rowStopIndex: stopIndex });
122-
focus(rootElement, `.bcp-item-row-${scrollToRow}`);
134+
if (focusOnRender) {
135+
focus(rootElement, `.bcp-item-row-${scrollToRow}`);
136+
}
123137
}}
124138
>
125139
<Column

‎src/components/KeyBinder.js‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ type Props = {
3838

3939
type State = {
4040
scrollToColumn: number,
41-
scrollToRow: number
41+
scrollToRow: number,
42+
focusOnRender: boolean
4243
};
4344

4445
class KeyBinder extends PureComponent<DefaultProps, Props, State> {
@@ -71,7 +72,8 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
7172

7273
this.state = {
7374
scrollToColumn: props.scrollToColumn,
74-
scrollToRow: props.scrollToRow
75+
scrollToRow: props.scrollToRow,
76+
focusOnRender: false
7577
};
7678

7779
this.columnStartIndex = 0;
@@ -87,18 +89,16 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
8789
* @inheritdoc
8890
* @return {void}
8991
*/
90-
componentWillReceiveProps(nextProps: Props) {
92+
componentWillReceiveProps(nextProps: Props): void {
9193
const { scrollToColumn, scrollToRow, currentCollection: { id } }: Props = nextProps;
92-
const {
93-
scrollToColumn: prevScrollToColumn,
94-
scrollToRow: prevScrollToRow,
95-
currentCollection: { id: prevId }
96-
}: Props = this.props;
94+
const { currentCollection: { id: prevId } }: Props = this.props;
95+
const { scrollToColumn: prevScrollToColumn, scrollToRow: prevScrollToRow }: State = this.state;
9796

9897
if (id !== prevId) {
9998
this.setState({
10099
scrollToColumn: 0,
101-
scrollToRow: 0
100+
scrollToRow: 0,
101+
focusOnRender: false
102102
});
103103
} else if (prevScrollToColumn !== scrollToColumn && prevScrollToRow !== scrollToRow) {
104104
this.setState({
@@ -119,7 +119,7 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
119119
* @inheritdoc
120120
* @return {void}
121121
*/
122-
onKeyDown = (event: SyntheticKeyboardEvent) => {
122+
onKeyDown = (event: SyntheticKeyboardEvent): void => {
123123
const {
124124
columnCount,
125125
rowCount,
@@ -141,6 +141,7 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
141141
switch (event.key) {
142142
case 'ArrowDown':
143143
scrollToRow = ctrlMeta ? rowCount - 1 : Math.min(scrollToRow + 1, rowCount - 1);
144+
event.stopPropagation(); // To prevent the arrow down capture of parent
144145
break;
145146
case 'ArrowLeft':
146147
scrollToColumn = ctrlMeta ? 0 : Math.max(scrollToColumn - 1, 0);
@@ -202,7 +203,7 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
202203
columnStopIndex: number,
203204
rowStartIndex: number,
204205
rowStopIndex: number
205-
}) => {
206+
}): void => {
206207
this.columnStartIndex = columnStartIndex;
207208
this.columnStopIndex = columnStopIndex;
208209
this.rowStartIndex = rowStartIndex;
@@ -216,10 +217,10 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
216217
* @inheritdoc
217218
* @return {void}
218219
*/
219-
updateScrollState({ scrollToColumn, scrollToRow }: State) {
220+
updateScrollState({ scrollToColumn, scrollToRow }: { scrollToColumn: number, scrollToRow: number }): void {
220221
const { onScrollToChange } = this.props;
221222
onScrollToChange({ scrollToColumn, scrollToRow });
222-
this.setState({ scrollToColumn, scrollToRow });
223+
this.setState({ scrollToColumn, scrollToRow, focusOnRender: true });
223224
}
224225

225226
/**
@@ -231,15 +232,16 @@ class KeyBinder extends PureComponent<DefaultProps, Props, State> {
231232
*/
232233
render() {
233234
const { className, children } = this.props;
234-
const { scrollToColumn, scrollToRow }: State = this.state;
235+
const { scrollToColumn, scrollToRow, focusOnRender }: State = this.state;
235236

236237
/* eslint-disable jsx-a11y/no-static-element-interactions */
237238
return (
238239
<div className={className} onKeyDown={this.onKeyDown}>
239240
{children({
240241
onSectionRendered: this.onSectionRendered,
241242
scrollToColumn,
242-
scrollToRow
243+
scrollToRow,
244+
focusOnRender
243245
})}
244246
</div>
245247
);

0 commit comments

Comments
 (0)