Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Commit

Permalink
Delay quick-open panel creation until used
Browse files Browse the repository at this point in the history 
Summary:
This diff reduces Nuclide's startup time by ~80ms. Most of this expense is the result of two forced layouts caused by quick-open: (1) measuring the document's height for the scrollable area `max-height` calculation, and (2) adding the panel to the DOM.

To fix #1, instead of setting the height in javascript of the scrollable area, it's set using CSS. The `max-height` is now `calc(100vh - GAPpx)`. Not only is this less taxing on Atom, but the modal now shrinks smoothly as you resize.

To fix #2, moved the panel creation code into the Activation class' render. Since that function got kinda big, I trimmed some of the unnecessary bits - like most of the styling on the modal parent.

Reviewed By: jgebhardt

Differential Revision: D3683543

fbshipit-source-id: 3b09bdcdd7f3aabb4e9162d68fbf6cdc44434f40
  • Loading branch information
@zertosh
zertosh authored and Facebook Github Bot 6 committed Aug 9, 2016
1 parent 730456d commit f3a446d
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 103 deletions.
9 changes: 7 additions & 2 deletions pkg/nuclide-quick-open/lib/QuickSelectionComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function sortServiceNames(names: Array<string>): Array<string> {

type Props = {
activeProvider: ProviderSpec,
maxScrollableAreaHeight?: number,
scrollableAreaHeightGap?: number,
onBlur: () => void,
};

Expand Down Expand Up @@ -806,7 +806,12 @@ export default class QuickSelectionComponent extends React.Component {
</Button>
</div>
{this._renderTabs()}
<div className="omnisearch-results" style={{maxHeight: this.props.maxScrollableAreaHeight}}>
<div
className="omnisearch-results"
style={{
maxHeight: this.props.scrollableAreaHeightGap ?
`calc(100vh - ${this.props.scrollableAreaHeightGap}px)` : '100vh',
}}>
{noResultsMessage}
<div className="omnisearch-pane">
<ul className="list-tree" ref="selectionList">
Expand Down
193 changes: 95 additions & 98 deletions pkg/nuclide-quick-open/lib/main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,62 +67,22 @@ const trackProviderChange = debounce(providerName => {
class Activation {
_currentProvider: Object;
_previousFocus: ?HTMLElement;
_reactDiv: HTMLElement;
_searchComponent: QuickSelectionComponent;
_searchPanel: atom$Panel;
_reactDiv: ?HTMLElement;
_searchComponent: ?QuickSelectionComponent;
_searchPanel: ?atom$Panel;
_subscriptions: atom$CompositeDisposable;
_debouncedUpdateModalPosition: () => void;
_maxScrollableAreaHeight: number;
_scrollableAreaHeightGap: number;

constructor() {
this._previousFocus = null;
this._maxScrollableAreaHeight = 10000;
this._scrollableAreaHeightGap = MODAL_MARGIN + TOPBAR_APPROX_HEIGHT;
this._subscriptions = new CompositeDisposable();
this._currentProvider = getSearchResultManager().getProviderByName(DEFAULT_PROVIDER);
QuickSelectionDispatcher.getInstance().register(action => {
if (action.actionType === QuickSelectionDispatcher.ActionType.ACTIVE_PROVIDER_CHANGED) {
this._handleActiveProviderChange(action.providerName);
}
});
this._reactDiv = document.createElement('div');
this._searchPanel = atom.workspace.addModalPanel({item: this._reactDiv, visible: false});
this._debouncedUpdateModalPosition = debounce(this._updateScrollableHeight.bind(this), 200);
window.addEventListener('resize', this._debouncedUpdateModalPosition);
this._customizeModalElement();
this._updateScrollableHeight();

this._searchComponent = this._render();

this._searchComponent.onSelection(selection => {
const options = {};
if (selection.line) {
options.initialLine = selection.line;
}
if (selection.column) {
options.initialColumn = selection.column;
}

atom.workspace.open(selection.path, options).then(textEditor => {
atom.commands.dispatch(atom.views.getView(textEditor), 'tabs:keep-preview-tab');
});

const query = this._searchComponent.getInputTextEditor().textContent;
const providerName = this._currentProvider.name;
// default to empty string because `track` enforces string-only values
const sourceProvider = selection.sourceProvider || '';
track(
AnalyticsEvents.SELECT_FILE,
{
'quickopen-filepath': selection.path,
'quickopen-query': query,
'quickopen-provider': providerName, // The currently open "tab".
'quickopen-session': analyticsSessionId || '',
// Because the `provider` is usually OmniSearch, also track the original provider.
'quickopen-provider-source': sourceProvider,
},
);
this.closeSearchPanel();
});

this._subscriptions.add(
atom.commands.add('body', 'core:cancel', () => {
Expand All @@ -132,61 +92,91 @@ class Activation {
}),
);

this._searchComponent.onCancellation(() => this.closeSearchPanel());
this._searchComponent.onSelectionChanged(debounce((selection: any) => {
// Only track user-initiated selection-change events.
if (analyticsSessionId != null) {
track(
AnalyticsEvents.CHANGE_SELECTION,
{
'quickopen-selected-index': selection.selectedItemIndex.toString(),
'quickopen-selected-service': selection.selectedService,
'quickopen-selected-directory': selection.selectedDirectory,
'quickopen-session': analyticsSessionId,
},
);
}
}, AnalyticsDebounceDelays.CHANGE_SELECTION));
}

// Customize the element containing the modal.
_customizeModalElement() {
const modalElement = ((this._searchPanel.getItem().parentNode: any): HTMLElement);
modalElement.style.setProperty('margin-left', '0');
modalElement.style.setProperty('max-width', 'none');
modalElement.style.setProperty('position', 'absolute');
modalElement.style.setProperty('width', 'auto');
modalElement.style.setProperty('left', MODAL_MARGIN + 'px');
modalElement.style.setProperty('right', MODAL_MARGIN + 'px');
(this: any).closeSearchPanel = this.closeSearchPanel.bind(this);
}

_updateScrollableHeight() {
const {height} = document.documentElement.getBoundingClientRect();
this._maxScrollableAreaHeight = height - MODAL_MARGIN - TOPBAR_APPROX_HEIGHT;
// Force a re-render to update _maxScrollableAreaHeight.
this._searchComponent = this._render();
}
_render(): void {
if (this._reactDiv == null) {
const _reactDiv = document.createElement('div');
this._searchPanel = atom.workspace.addModalPanel({
item: _reactDiv,
visible: false,
});
invariant(_reactDiv.parentNode instanceof HTMLElement);
_reactDiv.parentNode.style.maxWidth = `calc(100% - ${MODAL_MARGIN * 2}px)`;
this._reactDiv = _reactDiv;
}

_render(): QuickSelectionComponent {
const component = ReactDOM.render(
const _searchComponent = ReactDOM.render(
<QuickSelectionComponent
activeProvider={this._currentProvider}
maxScrollableAreaHeight={this._maxScrollableAreaHeight}
onBlur={this.closeSearchPanel.bind(this)}
scrollableAreaHeightGap={this._scrollableAreaHeightGap}
onBlur={this.closeSearchPanel}
/>,
this._reactDiv,
);
invariant(component instanceof QuickSelectionComponent);
return component;
invariant(_searchComponent instanceof QuickSelectionComponent);

if (this._searchComponent == null) {
_searchComponent.onSelection(selection => {
const options = {};
if (selection.line) {
options.initialLine = selection.line;
}
if (selection.column) {
options.initialColumn = selection.column;
}

atom.workspace.open(selection.path, options).then(textEditor => {
atom.commands.dispatch(atom.views.getView(textEditor), 'tabs:keep-preview-tab');
});

const query = _searchComponent.getInputTextEditor().textContent;
const providerName = this._currentProvider.name;
// default to empty string because `track` enforces string-only values
const sourceProvider = selection.sourceProvider || '';
track(
AnalyticsEvents.SELECT_FILE,
{
'quickopen-filepath': selection.path,
'quickopen-query': query,
'quickopen-provider': providerName, // The currently open "tab".
'quickopen-session': analyticsSessionId || '',
// Because the `provider` is usually OmniSearch, also track the original provider.
'quickopen-provider-source': sourceProvider,
},
);
this.closeSearchPanel();
});

_searchComponent.onCancellation(() => this.closeSearchPanel());
_searchComponent.onSelectionChanged(debounce((selection: any) => {
// Only track user-initiated selection-change events.
if (analyticsSessionId != null) {
track(
AnalyticsEvents.CHANGE_SELECTION,
{
'quickopen-selected-index': selection.selectedItemIndex.toString(),
'quickopen-selected-service': selection.selectedService,
'quickopen-selected-directory': selection.selectedDirectory,
'quickopen-session': analyticsSessionId,
},
);
}
}, AnalyticsDebounceDelays.CHANGE_SELECTION));
}

this._searchComponent = _searchComponent;
}


_handleActiveProviderChange(newProviderName: string): void {
trackProviderChange(newProviderName);
// Toggle newProviderName before setting this._currentProvider to make
// the search panel stay open.
this.toggleProvider(newProviderName);
this._currentProvider = getSearchResultManager().getProviderByName(newProviderName);
this._searchComponent = this._render();
this._render();
}

toggleOmniSearchProvider(): void {
Expand All @@ -205,7 +195,7 @@ class Activation {
const provider = getSearchResultManager().getProviderByName(providerName);
// "toggle" behavior
if (
this._searchPanel !== null &&
this._searchPanel != null &&
this._searchPanel.isVisible() &&
providerName === this._currentProvider.name
) {
Expand All @@ -214,15 +204,14 @@ class Activation {
}

this._currentProvider = provider;
if (this._searchComponent) {
this._searchComponent = this._render();
}
this._render();
this.showSearchPanel();
}

showSearchPanel() {
this._previousFocus = document.activeElement;
if (this._searchComponent && this._searchPanel) {
const {_searchComponent, _searchPanel} = this;
if (_searchComponent != null && _searchPanel != null) {
// Start a new search "session" for analytics purposes.
track(
AnalyticsEvents.OPEN_PANEL,
Expand All @@ -231,29 +220,30 @@ class Activation {
},
);
// showSearchPanel gets called when changing providers even if it's already shown.
const isAlreadyVisible = this._searchPanel.isVisible();
this._searchPanel.show();
this._searchComponent.focus();
const isAlreadyVisible = _searchPanel.isVisible();
_searchPanel.show();
_searchComponent.focus();
if (featureConfig.get('nuclide-quick-open.useSelection') && !isAlreadyVisible) {
const selectedText = this._getFirstSelectionText();
if (selectedText && selectedText.length <= MAX_SELECTION_LENGTH) {
this._searchComponent.setInputValue(selectedText.split('\n')[0]);
_searchComponent.setInputValue(selectedText.split('\n')[0]);
}
}
this._searchComponent.selectInput();
_searchComponent.selectInput();
}
}

closeSearchPanel() {
if (this._searchComponent && this._searchPanel) {
const {_searchComponent, _searchPanel} = this;
if (_searchComponent != null && _searchPanel != null) {
track(
AnalyticsEvents.CLOSE_PANEL,
{
'quickopen-session': analyticsSessionId || '',
},
);
this._searchPanel.hide();
this._searchComponent.blur();
_searchPanel.hide();
_searchComponent.blur();
analyticsSessionId = null;
}

Expand All @@ -272,7 +262,14 @@ class Activation {

dispose(): void {
this._subscriptions.dispose();
ReactDOM.unmountComponentAtNode(this._reactDiv);
if (this._reactDiv != null) {
ReactDOM.unmountComponentAtNode(this._reactDiv);
this._reactDiv = null;
}
if (this._searchPanel != null) {
this._searchPanel.destroy();
this._searchPanel = null;
}
}
}

Expand Down
16 changes: 13 additions & 3 deletions spec/utils/quick-open-provider-cycle-common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,20 @@ export function runTest(context: TestContext) {
return omniSearchTreeNodes.length > 0;
});

// Expect an item to exist
waitsFor('first result item is selected', () => {
firstActiveElement =
document.querySelector('.quick-open-result-item.list-item.selected:first-child');
return firstActiveElement != null;
});

// Expect that 'down arrow' selects the next item
runs(() => {
firstActiveElement = document.querySelector('.quick-open-result-item.list-item.selected');
expect(firstActiveElement).not.toBeNull();
dispatchKeyboardEvent('down', document.activeElement);
const nextActiveElement =
document.querySelector('.quick-open-result-item.list-item.selected');
expect(nextActiveElement).not.toBeNull();
invariant(firstActiveElement != null);
expect(nextActiveElement).toBe(firstActiveElement.nextElementSibling);
});

waitsFor('active quick-open item to scroll back to first element', () => {
Expand Down

0 comments on commit f3a446d

Please sign in to comment.