Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Use SelectControl in Context Picker Modal #8688

Merged
merged 3 commits into from
Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 86 additions & 43 deletions src/sentry/static/sentry/app/components/contextPickerModal.jsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import PropTypes from 'prop-types';
import React from 'react';
import ReactDOM from 'react-dom';
import Reflux from 'reflux';
import createReactClass from 'create-react-class';
import styled from 'react-emotion';

import {fetchOrganizationDetails} from 'app/actionCreators/organizations';
import {t} from 'app/locale';
import LatestContextStore from 'app/stores/latestContextStore';
import LoadingIndicator from 'app/components/loadingIndicator';
import OrganizationsStore from 'app/stores/organizationsStore';
import Select2Field from 'app/components/forms/select2Field';
import SelectControl from 'app/components/forms/selectControl';
import SentryTypes from 'app/proptypes';
import replaceRouterParams from 'app/utils/replaceRouterParams';
import withProjects from 'app/utils/withProjects';
import space from 'app/styles/space';

class ContextPickerModal extends React.Component {
static propTypes = {
Expand Down Expand Up @@ -62,10 +65,19 @@ class ContextPickerModal extends React.Component {
constructor(props) {
super(props);

let {needProject, latestContext} = props;

let shouldHaveEmptyOrgSelector = !needProject || !latestContext.organization;
this.state = {
// Initialize loading to true if there is only 1 organization because component will immediately
// attempt to fetch org details for that org. Otherwise we'd have to change state in `DidMount`
loading: props.organizations.length === 1,

// Org select should be empty except if we need a project and there's an org in context
// (otherwise they need to select an org before we can fetch projects)
selectedOrganization: shouldHaveEmptyOrgSelector
? ''
: latestContext.organization.slug,
};
}

Expand Down Expand Up @@ -160,7 +172,28 @@ class ContextPickerModal extends React.Component {
);
};

handleSelectOrganization = value => {
focusProjectSelector = () => {
if (!this.projectSelect || this.state.loading) return;

// eslint-disable-next-line react/no-find-dom-node
ReactDOM.findDOMNode(this.projectSelect)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use forwardRef to clean this up, but there's some issues with react-test-renderer serializing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.querySelector('input')
.focus();
};

focusOrganizationSelector = () => {
if (!this.orgSelect || this.state.loading) return;

// eslint-disable-next-line react/no-find-dom-node
ReactDOM.findDOMNode(this.orgSelect)
.querySelector('input')
.focus();
};

handleSelectOrganization = ({value}) => {
// Don't do anything if org value doesn't actually change
if (this.state.selectedOrganization === value) return;

// If we do not need to select a project, we can early return after selecting an org
// No need to fetch org details
if (!this.props.needProject) {
Expand All @@ -170,18 +203,19 @@ class ContextPickerModal extends React.Component {

this.setState(
{
selectedOrganization: value,
loading: true,
},
() => fetchOrganizationDetails(value, {setActive: true, loadProjects: true})
);
};

handleSelectProject = projectId => {
handleSelectProject = ({value}) => {
let {latestContext} = this.props;

if (!projectId || !latestContext.organization) return;
if (!value || !latestContext.organization) return;

this.navigateIfFinish([latestContext.organization], [{slug: projectId}]);
this.navigateIfFinish([latestContext.organization], [{slug: value}]);
};

render() {
Expand All @@ -194,52 +228,53 @@ class ContextPickerModal extends React.Component {

if (!shouldShowPicker) return null;

// Org select should be empty except if we need a project and there's an org in context
// (otherwise they need to select an org before we can fetch projects)
let shouldHaveEmptyOrgSelector = !needProject || !latestContext.organization;
let shouldShowProjectSelector = latestContext.organization && needProject && projects;

// We're inserting a blank el for `select2` so that we can have the placeholder :(
let orgChoices = organizations
.filter(({status}) => status.id !== 'pending_deletion')
.map(({slug}) => [slug, slug]);
.map(({slug}) => ({label: slug, value: slug}));

return (
<div>
{loading && <LoadingIndicator />}
{!loading && (
<React.Fragment>
<Header closeButton>{t('Select...')}</Header>
<Body>
<div css={{marginBottom: 12}}>
{t('Select an organization/project to continue')}
</div>
{needOrg && (
<Select2Field
placeholder="Select an Organization"
name="organization"
choices={[['', ''], ...orgChoices]}
value={
shouldHaveEmptyOrgSelector ? '' : latestContext.organization.slug
}
onChange={this.handleSelectOrganization}
<React.Fragment>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't need this

<Header closeButton>{t('Select...')}</Header>
<Body ref={this.handleBodyMount}>
{loading && <StyledLoadingIndicator overlay />}
<div>{t('Select an organization/project to continue')}</div>
{needOrg && (
<StyledSelectControl
innerRef={ref => {
this.orgSelect = ref;
if (shouldShowProjectSelector) return;
this.focusOrganizationSelector();
}}
placeholder="Select an Organization"
name="organization"
options={orgChoices}
openOnFocus
value={this.state.selectedOrganization}
onChange={this.handleSelectOrganization}
/>
)}

{latestContext.organization &&
needProject &&
projects && (
<StyledSelectControl
innerRef={ref => {
this.projectSelect = ref;
this.focusProjectSelector();
}}
placeholder="Select a Project"
name="project"
value=""
openOnFocus
options={projects.map(({slug}) => ({label: slug, value: slug}))}
onChange={this.handleSelectProject}
/>
)}

{latestContext.organization &&
needProject &&
projects && (
<Select2Field
placeholder="Select a Project"
name="project"
allowEmpty
value=""
choices={[['', ''], ...projects.map(({slug}) => [slug, slug])]}
onChange={this.handleSelectProject}
/>
)}
</Body>
</React.Fragment>
)}
</Body>
</React.Fragment>
</div>
);
}
Expand All @@ -266,3 +301,11 @@ const ContextPickerModalContainer = withProjects(

export default ContextPickerModalContainer;
export {ContextPickerModal};

const StyledSelectControl = styled(SelectControl)`
margin-top: ${space(1)};
`;

const StyledLoadingIndicator = styled(LoadingIndicator)`
z-index: 1;
`;
15 changes: 14 additions & 1 deletion src/sentry/static/sentry/app/components/globalModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ class GlobalModal extends React.Component {
modalClassName: PropTypes.string,
}),
visible: PropTypes.bool,
/**
* Note this is the callback for the main App container and
* NOT the calling component. GlobalModal is never used directly,
* but is controlled via stores. To access the onClose callback from
* the component, you must specify it when using the action creator.
*/
onClose: PropTypes.func,
};

static defaultProps = {
Expand All @@ -32,14 +39,20 @@ class GlobalModal extends React.Component {
};

handleCloseModal = () => {
let {options} = this.props;
let {options, onClose} = this.props;

// onClose callback for calling component
if (typeof options.onClose === 'function') {
options.onClose();
}

// Action creator
closeModal();

// Read description in propTypes
if (typeof onClose === 'function') {
onClose();
}
};

render() {
Expand Down
16 changes: 14 additions & 2 deletions src/sentry/static/sentry/app/views/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ const App = createReactClass({
});
},

handleGlobalModalClose() {
if (!this.mainContainerRef) return;
if (typeof this.mainContainerRef.focus !== 'function') return;

// Focus the main container to get hotkeys to keep working after modal closes
this.mainContainerRef.focus();
},

renderBody() {
let {needsUpgrade, newsletterConsentPrompt} = this.state;
if (needsUpgrade) {
Expand All @@ -185,8 +193,12 @@ const App = createReactClass({

return (
<ThemeProvider theme={theme}>
<div className="main-container" tabIndex="-1">
<GlobalModal />
<div
className="main-container"
tabIndex="-1"
ref={ref => (this.mainContainerRef = ref)}
>
<GlobalModal onClose={this.handleGlobalModalClose} />
<Alerts className="messages-container" />
<Indicators className="indicators-container" />
<ErrorBoundary>{this.renderBody()}</ErrorBoundary>
Expand Down
58 changes: 35 additions & 23 deletions tests/js/spec/components/contextPickerModal.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ describe('ContextPickerModal', function() {
it('renders with only org selector when no org in latest context', function() {
let wrapper = shallow(getComponent());

expect(wrapper.find('Select2Field[name="organization"]').exists()).toBe(true);
expect(wrapper.find('Select2Field[name="project"]').exists()).toBe(false);
expect(wrapper.find('StyledSelectControl[name="organization"]').exists()).toBe(true);
expect(wrapper.find('StyledSelectControl[name="project"]').exists()).toBe(false);
});

it('fetches org details and sets as active org if there is only one org', function() {
Expand Down Expand Up @@ -107,10 +107,14 @@ describe('ContextPickerModal', function() {
url: `/organizations/${org.slug}/`,
});

wrapper.find('StyledSelectControl[name="organization"] input').simulate('focus');

expect(wrapper.find('Select[name="organization"] .Select-menu')).toHaveLength(1);

wrapper
.find('Select2Field[name="organization"]')
.instance()
.onChange({target: {value: org.slug}});
.find('Select[name="organization"] Option')
.first()
.simulate('mouseDown');
expect(onFinish).toHaveBeenCalledWith('/test/org-slug/path/');
// Is not called because we don't need to fetch org details
expect(mock).not.toHaveBeenCalled();
Expand All @@ -131,17 +135,16 @@ describe('ContextPickerModal', function() {
);

// Default to org in latest context
expect(wrapper.find('Select2Field[name="organization"]').prop('value')).toBe(
expect(wrapper.find('StyledSelectControl[name="organization"]').prop('value')).toBe(
org.slug
);
expect(wrapper.find('Select2Field[name="project"]').prop('choices')).toEqual([
['', ''],
[project.slug, project.slug],
[project2.slug, project2.slug],
expect(wrapper.find('StyledSelectControl[name="project"]').prop('options')).toEqual([
{value: project.slug, label: project.slug},
{value: project2.slug, label: project2.slug},
]);
});

it('can select org and project', function() {
it('can select org and project', async function() {
let spy = jest.spyOn(OrgActions, 'fetchOrganizationDetails');
let api = MockApiClient.addMockResponse({
url: `/organizations/${org2.slug}/`,
Expand All @@ -165,13 +168,22 @@ describe('ContextPickerModal', function() {
);

// Should not have anything selected
expect(wrapper.find('Select2Field[name="organization"]').prop('value')).toBe('');
expect(wrapper.find('StyledSelectControl[name="organization"]').prop('value')).toBe(
''
);

spy.mockClear();

// Select org2
wrapper
.find('Select2Field[name="organization"]')
.instance()
.onChange({target: {value: org2.slug}});
.find('StyledSelectControl[name="organization"]')
.simulate('change', {value: org2.slug, label: org2.slug});

wrapper.find('StyledSelectControl[name="organization"] input').simulate('focus');
wrapper
.find('Select[name="organization"] Option')
.at(1)
.simulate('mouseDown');

expect(spy).toHaveBeenCalledWith('org2', {
setActive: true,
Expand All @@ -185,17 +197,17 @@ describe('ContextPickerModal', function() {
});
wrapper.update();

expect(wrapper.find('Select2Field[name="project"]').prop('choices')).toEqual([
['', ''],
[project2.slug, project2.slug],
['project3', 'project3'],
expect(wrapper.find('StyledSelectControl[name="project"]').prop('options')).toEqual([
{value: project2.slug, label: project2.slug},
{value: 'project3', label: 'project3'},
]);

// Select org3
// Select project3
wrapper.find('StyledSelectControl[name="project"] input').simulate('focus');
wrapper
.find('Select2Field[name="project"]')
.instance()
.onChange({target: {value: 'project3'}});
.find('Select[name="project"] Option')
.at(1)
.simulate('mouseDown');

expect(onFinish).toHaveBeenCalledWith('/test/org2/path/project3/');
});
Expand Down