Unverified Commit acc7f815 authored by Imran Salam's avatar Imran Salam Committed by GitHub
Browse files

Enhancements and new functionality for navigation bar (#162)

* add region editor to nav bar and remove browser genome selector

* use the Input component for input text field in browser region field

* change genomeSelectorActive state property to browserRegionEditorActive

* add region editor and make improvements to region field

* fix styling issues in browser nav bar

* validate browser region field with api

* fix region field error messages not disappearing after closing

* add validation to browser region editor

* use constants for browser region errors

* fix paths to shared components after merge

* improve types in browser state

* remove redundanta browser navigator button

* rename browser region field input

* add test cases for browser region editor

* add test cases for browser region field

* Design review changes

* fix test cases in browser nav bar

* fix wrong import path for loading state

* remove redundant dispatch call

* fix multiple dispatch calls made when user focuses on region input

* fix location update issues on region form submissions

* fix test cases

* added temporary base url of the endpoints used for region input forms

* fix type issues

* enable region field submissions with no region (location only)

* merge with dev to get the changes made to browser

* make changes as suggested in webteam meeting and fix bugs

* fix tests

* fix tests after changes made to validation

* remove spaces from the hyphen between location start and end

* make the region field placeholder paler in colour

* fix handlebars package security vulnerability

* add tests for browser components

* add tests to browser zmenu components

* update region validation code to work with the validation endpoint changes

* add tests to browser cog components and move them to a separate directory

* remove unused objects and types in browser config

* add test cases for browser helper functions

* update tests for browser field and region inputs

* PR review suggestions

* More PR review suggestions

* Even more PR review suggestions

* fix more issues in PR

* fix genome browser disappearing when navigation panel is opened

* fix karyotype name and add types to region validation result

* remove redundant loading state import and fix regression issue in browser bar

* add parse error to browser region validation and sort region validation types

* fix test case description for browser helper

* reset the api proxy url for webpack to staging

* fix region editor select menu not being clickable
parent b744dddc
Pipeline #43289 passed with stages
in 4 minutes and 26 seconds
......@@ -25,15 +25,6 @@
height: 100%;
}
.browserOverlay {
background: $white;
height: 100%;
opacity: 0.7;
position: absolute;
width: 100vw;
z-index: 150;
}
.exampleLinks {
margin-top: 50px;
margin-left: 40px;
......@@ -55,3 +46,7 @@
.objectLabel {
color: $blue;
}
.semiOpaque {
opacity: 0.3;
}
describe.skip('<Browser />', () => {
test('it works', () => {
// this is a reminder to test the Browser component
import React from 'react';
import { MemoryRouter } from 'react-router';
import { mount } from 'enzyme';
import faker from 'faker';
import { Browser, BrowserProps, ExampleObjectLinks } from './Browser';
import BrowserImage from './browser-image/BrowserImage';
import TrackPanel from './track-panel/TrackPanel';
import { createChrLocationValues } from 'tests/fixtures/browser';
jest.mock('./browser-bar/BrowserBar', () => () => <div>BrowserBar</div>);
jest.mock('./browser-image/BrowserImage', () => () => <div>BrowserImage</div>);
jest.mock('./browser-nav/BrowserNavBar', () => () => <div>BrowserNavBar</div>);
jest.mock('./track-panel/TrackPanel', () => () => <div>TrackPanel</div>);
jest.mock('./browser-app-bar/BrowserAppBar', () => () => (
<div>BrowserAppBar</div>
));
jest.mock('static/browser/browser.js', () => {});
describe('<Browser />', () => {
afterEach(() => {
jest.resetAllMocks();
});
const defaultProps: BrowserProps = {
activeGenomeId: faker.lorem.words(),
activeEnsObjectId: faker.lorem.words(),
allActiveEnsObjectIds: {
[faker.lorem.words()]: faker.lorem.words()
},
allChrLocations: {
[faker.lorem.words()]: createChrLocationValues().tupleValue
},
browserActivated: false,
browserNavOpened: false,
browserQueryParams: {},
chrLocation: createChrLocationValues().tupleValue,
isDrawerOpened: false,
isTrackPanelOpened: false,
launchbarExpanded: false,
exampleEnsObjects: [],
committedSpecies: [],
changeBrowserLocation: jest.fn(),
changeFocusObject: jest.fn(),
changeDrawerView: jest.fn(),
closeDrawer: jest.fn(),
restoreBrowserTrackStates: jest.fn(),
fetchGenomeData: jest.fn(),
replace: jest.fn(),
toggleDrawer: jest.fn(),
setDataFromUrlAndSave: jest.fn()
};
const wrappingComponent = (props: any) => (
<MemoryRouter>{props.children}</MemoryRouter>
);
const mountBrowserComponent = (props?: Partial<BrowserProps>) =>
mount(<Browser {...defaultProps} {...props} />, { wrappingComponent });
describe('rendering', () => {
test('does not render when no activeGenomeId', () => {
const wrapper = mountBrowserComponent({ activeGenomeId: null });
expect(wrapper.html()).toBe(null);
});
test('renders links to example objects only if there is no selected focus feature', () => {
const wrapper = mountBrowserComponent();
expect(wrapper.find(ExampleObjectLinks)).toHaveLength(1);
wrapper.setProps({
browserQueryParams: {
focus: faker.lorem.words()
}
});
expect(wrapper.find(ExampleObjectLinks)).toHaveLength(0);
});
test('renders the genome browser and track panel only when there is a selected focus feature', () => {
const wrapper = mountBrowserComponent();
expect(wrapper.find(BrowserImage)).toHaveLength(0);
expect(wrapper.find(TrackPanel)).toHaveLength(0);
wrapper.setProps({
browserQueryParams: { focus: faker.lorem.words() }
});
expect(wrapper.find(BrowserImage)).toHaveLength(1);
expect(wrapper.find(TrackPanel)).toHaveLength(1);
});
});
describe('behaviour', () => {
test('fetches genome data when selected genome changes', () => {
const wrapper = mountBrowserComponent({ activeGenomeId: null });
expect(wrapper.props().fetchGenomeData).toHaveBeenCalledTimes(0);
wrapper.setProps({ activeGenomeId: faker.lorem.words() });
expect(wrapper.props().fetchGenomeData).toHaveBeenCalledTimes(1);
wrapper.setProps({ activeGenomeId: faker.lorem.words() });
expect(wrapper.props().fetchGenomeData).toHaveBeenCalledTimes(2);
});
test('closes drawer when clicked on genome browser area', () => {
const wrapper = mountBrowserComponent({
browserQueryParams: { focus: faker.lorem.words() },
isDrawerOpened: true,
isTrackPanelOpened: true
});
wrapper.find('.browserImageWrapper').simulate('click');
expect(wrapper.props().closeDrawer).toHaveBeenCalled();
});
});
});
import React, { FunctionComponent, useEffect, useState } from 'react';
import { withRouter, RouteComponentProps } from 'react-router-dom';
import React, { useEffect, useState } from 'react';
import { useLocation, useParams } from 'react-router-dom';
import { connect } from 'react-redux';
import { replace, Replace } from 'connected-react-router';
import { useSpring, animated } from 'react-spring';
......@@ -26,7 +26,6 @@ import {
import {
getBrowserNavOpened,
getChrLocation,
getGenomeSelectorActive,
getBrowserActivated,
getBrowserActiveGenomeId,
getBrowserQueryParams,
......@@ -59,7 +58,7 @@ import styles from './Browser.scss';
import 'static/browser/browser.js';
type StateProps = {
export type BrowserProps = {
activeGenomeId: string | null;
activeEnsObjectId: string | null;
allActiveEnsObjectIds: { [genomeId: string]: string };
......@@ -68,15 +67,11 @@ type StateProps = {
browserNavOpened: boolean;
browserQueryParams: { [key: string]: string };
chrLocation: ChrLocation | null;
genomeSelectorActive: boolean;
isDrawerOpened: boolean;
isTrackPanelOpened: boolean;
launchbarExpanded: boolean;
exampleEnsObjects: EnsObject[];
committedSpecies: CommittedItem[];
};
type DispatchProps = {
changeBrowserLocation: (locationData: {
genomeId: string;
ensObjectId: string | null;
......@@ -92,28 +87,17 @@ type DispatchProps = {
setDataFromUrlAndSave: (payload: ParsedUrlPayload) => void;
};
type OwnProps = {};
type MatchParams = {
genomeId: string;
};
type BrowserProps = RouteComponentProps<MatchParams> &
StateProps &
DispatchProps &
OwnProps;
export const Browser: FunctionComponent<BrowserProps> = (
props: BrowserProps
) => {
export const Browser = (props: BrowserProps) => {
const [trackStatesFromStorage, setTrackStatesFromStorage] = useState<
BrowserTrackStates
>({});
const { isDrawerOpened, closeDrawer } = props;
const params: { [key: string]: string } = useParams();
const location = useLocation();
const setDataFromUrl = () => {
const { genomeId = null } = props.match.params;
const { genomeId } = params;
const { focus = null, location = null } = props.browserQueryParams;
const chrLocation = location ? getChrLocationFromStr(location) : null;
......@@ -174,7 +158,7 @@ export const Browser: FunctionComponent<BrowserProps> = (
// handle url changes
useEffect(() => {
// handle navigation to /app/browser
if (!props.match.params.genomeId) {
if (!params.genomeId) {
// select either the species that the user viewed during the previous visit,
// of the first selected species
const { activeGenomeId, committedSpecies } = props;
......@@ -195,7 +179,7 @@ export const Browser: FunctionComponent<BrowserProps> = (
// handle navigation to /app/browser/:genomeId?focus=:focus&location=:location
setDataFromUrl();
}
}, [props.match.params.genomeId, props.location.search]);
}, [params.genomeId, location.search]);
useEffect(() => {
const { activeGenomeId, fetchGenomeData } = props;
......@@ -213,11 +197,9 @@ export const Browser: FunctionComponent<BrowserProps> = (
useEffect(() => {
const {
match: {
params: { genomeId }
},
browserQueryParams: { location }
} = props;
const { genomeId } = params;
const chrLocation = location ? getChrLocationFromStr(location) : null;
if (props.browserActivated && genomeId && chrLocation) {
......@@ -263,7 +245,11 @@ export const Browser: FunctionComponent<BrowserProps> = (
const shouldShowNavBar =
props.browserActivated && props.browserNavOpened && !isDrawerOpened;
return props.activeGenomeId ? (
if (!props.activeGenomeId) {
return null;
}
return (
<>
<BrowserAppBar onSpeciesSelect={changeSelectedSpecies} />
{!props.browserQueryParams.focus && (
......@@ -272,12 +258,9 @@ export const Browser: FunctionComponent<BrowserProps> = (
<ExampleObjectLinks {...props} />
</section>
)}
{props.browserQueryParams.focus && (
{!!props.browserQueryParams.focus && (
<section className={styles.browser}>
{browserBar}
{props.genomeSelectorActive && (
<div className={styles.browserOverlay} />
)}
<div
className={`${styles.browserInnerWrapper} ${getHeightClass(
props.launchbarExpanded
......@@ -286,7 +269,7 @@ export const Browser: FunctionComponent<BrowserProps> = (
<animated.div style={trackAnimation}>
<div className={styles.browserImageWrapper} onClick={closeTrack}>
{shouldShowNavBar && <BrowserNavBar />}
<BrowserImage trackStates={trackStatesFromStorage} />
<BrowserImage />
</div>
</animated.div>
<TrackPanel />
......@@ -294,14 +277,16 @@ export const Browser: FunctionComponent<BrowserProps> = (
</section>
)}
</>
) : null;
);
};
const ExampleObjectLinks = (props: BrowserProps) => {
export const ExampleObjectLinks = (props: BrowserProps) => {
const { activeGenomeId } = props;
if (!activeGenomeId) {
return null;
}
const links = props.exampleEnsObjects.map((exampleObject: EnsObject) => {
const path = urlFor.browser({
genomeId: activeGenomeId,
......@@ -323,7 +308,7 @@ const ExampleObjectLinks = (props: BrowserProps) => {
return <div className={styles.exampleLinks}>{links}</div>;
};
const mapStateToProps = (state: RootState): StateProps => ({
const mapStateToProps = (state: RootState) => ({
activeGenomeId: getBrowserActiveGenomeId(state),
activeEnsObjectId: getBrowserActiveEnsObjectId(state),
allActiveEnsObjectIds: getBrowserActiveEnsObjectIds(state),
......@@ -332,7 +317,6 @@ const mapStateToProps = (state: RootState): StateProps => ({
browserNavOpened: getBrowserNavOpened(state),
browserQueryParams: getBrowserQueryParams(state),
chrLocation: getChrLocation(state),
genomeSelectorActive: getGenomeSelectorActive(state),
isDrawerOpened: getIsDrawerOpened(state),
isTrackPanelOpened: getIsTrackPanelOpened(state),
launchbarExpanded: getLaunchbarExpanded(state),
......@@ -340,7 +324,7 @@ const mapStateToProps = (state: RootState): StateProps => ({
committedSpecies: getEnabledCommittedSpecies(state)
});
const mapDispatchToProps: DispatchProps = {
const mapDispatchToProps = {
changeBrowserLocation,
changeFocusObject,
changeDrawerView,
......@@ -352,9 +336,7 @@ const mapDispatchToProps: DispatchProps = {
restoreBrowserTrackStates
};
export default withRouter(
connect(
mapStateToProps,
mapDispatchToProps
)(Browser)
);
export default connect(
mapStateToProps,
mapDispatchToProps
)(Browser);
......@@ -88,3 +88,48 @@
}
}
}
.browserInfoRegion {
.chrLabel {
display: inline-block;
margin-right: 5px;
}
span {
cursor: pointer;
}
}
.chrLocationView {
display: inline-block;
}
.chrCode {
background: $dark-blue;
color: $white;
cursor: pointer;
display: inline-block;
font-weight: $bold;
margin: 0 5px;
padding: 1px 4px;
text-align: center;
}
.chrRegion {
color: $dark-blue;
display: inline-block;
}
.browserInfoHidden {
.chrCode {
background: $dark-grey;
}
.chrRegion {
color: $dark-grey;
}
}
.chrSeparator {
margin: 0 2px;
}
import React from 'react';
import { mount } from 'enzyme';
import faker from 'faker';
import {
BrowserBar,
BrowserInfo,
BrowserNavigatorButton,
BrowserBarProps
} from './BrowserBar';
import { BrowserBar, BrowserInfo, BrowserBarProps } from './BrowserBar';
import { BreakpointWidth } from 'src/global/globalConfig';
import BrowserReset from 'src/content/app/browser/browser-reset/BrowserReset';
import BrowserGenomeSelector from 'src/content/app/browser/browser-genome-selector/BrowserGenomeSelector';
import TrackPanelTabs from '../track-panel/track-panel-tabs/TrackPanelTabs';
import { ChrLocation } from '../browserState';
......@@ -22,10 +16,6 @@ import { createEnsObject } from 'tests/fixtures/ens-object';
jest.mock('src/content/app/browser/browser-reset/BrowserReset', () => () => (
<div>BrowserReset</div>
));
jest.mock(
'src/content/app/browser/browser-genome-selector/BrowserGenomeSelector',
() => () => <div>BrowserGenomeSelector</div>
);
jest.mock('../track-panel/track-panel-tabs/TrackPanelTabs', () => () => (
<div>BrowserReset</div>
));
......@@ -34,7 +24,6 @@ describe('<BrowserBar />', () => {
const selectTrackPanelTab: any = jest.fn();
const toggleBrowserNav: any = jest.fn();
const toggleDrawer: any = jest.fn();
const toggleGenomeSelector: any = jest.fn();
const defaultProps = {
activeGenomeId: faker.lorem.word(),
......@@ -45,7 +34,6 @@ describe('<BrowserBar />', () => {
actualChrLocation: ['13', 32275301, 32433493] as ChrLocation,
defaultChrLocation: ['13', 32271473, 32437359] as ChrLocation,
drawerOpened: false,
genomeSelectorActive: false,
ensObject: createEnsObject(),
selectedTrackPanelTab: TrackSet.GENOMIC,
trackPanelModalOpened: false,
......@@ -53,7 +41,6 @@ describe('<BrowserBar />', () => {
selectTrackPanelTab,
toggleBrowserNav,
toggleDrawer,
toggleGenomeSelector,
isDrawerOpened: false,
isTrackPanelModalOpened: false,
isTrackPanelOpened: false,
......@@ -85,10 +72,6 @@ describe('<BrowserBar />', () => {
test('contains BrowserReset button', () => {
expect(renderedBrowserBar.find(BrowserReset)).toHaveLength(1);
});
test('contains BrowserGenomeSelector', () => {
expect(renderedBrowserBar.find(BrowserGenomeSelector)).toHaveLength(1);
});
});
describe('behaviour', () => {
......@@ -104,34 +87,6 @@ describe('<BrowserBar />', () => {
expect(renderedBrowserBar.find(BrowserInfo).length).toBe(0);
});
test('hides BrowserInfo panel if genome selector becomes active', async () => {
const renderedBrowserBar = mount(renderBrowserBar());
renderedBrowserBar.setProps({ genomeSelectorActive: true });
// ugly hack: fall back to the end of event queue, giving priority to useEffect and useState
await new Promise((resolve) => setTimeout(resolve, 0));
renderedBrowserBar.update();
expect(renderedBrowserBar.find(BrowserInfo).length).toBe(0);
});
test('shows BrowserNavigatorButton if genomeSelector is not active', () => {
const renderedBrowserBar = mount(renderBrowserBar());
expect(renderedBrowserBar.find(BrowserNavigatorButton).length).toBe(1);
});
test('hides BrowserNavigatorButton if genomeSelector is active', () => {
const renderedBrowserBar = mount(
renderBrowserBar({ genomeSelectorActive: true })
);
expect(renderedBrowserBar.find(BrowserNavigatorButton).length).toBe(0);
});
test('hides BrowserNavigatorButton if there is no focus object', () => {
const renderedBrowserBar = mount(renderBrowserBar({ ensObject: null }));
expect(renderedBrowserBar.find(BrowserNavigatorButton).length).toBe(0);
});
test('shows TrackPanelTabs if TrackPanel is open', () => {
const renderedBrowserBar = mount(
renderBrowserBar({ isTrackPanelOpened: true })
......@@ -144,7 +99,7 @@ describe('<BrowserBar />', () => {
expect(renderedBrowserBar.find(TrackPanelTabs).length).toBe(1);
});
test('hides TrackPanelTabs on small if TrackPanel is closed', () => {
test('hides TrackPanelTabs on a small display if TrackPanel is closed', () => {
const renderedBrowserBar = mount(
renderBrowserBar({ breakpointWidth: BreakpointWidth.LAPTOP })
);
......
import React, { FunctionComponent, useState, useEffect } from 'react';
import React, { useState, useEffect } from 'react';
import { connect } from 'react-redux';
import classNames from 'classnames';
import { browserInfoConfig, BrowserInfoItem } from '../browserConfig';
import { TrackSet } from '../track-panel/trackPanelConfig';
import { BreakpointWidth } from 'src/global/globalConfig';
import { EnsObject } from 'src/ens-object/ensObjectTypes';
import { getDisplayStableId } from 'src/ens-object/ensObjectHelpers';
import { getFormattedLocation } from 'src/shared/helpers/regionFormatter';
import { getCommaSeparatedNumber } from 'src/shared/helpers/numberFormatter';
import {
toggleBrowserNav,
toggleGenomeSelector,
changeFocusObject
} from '../browserActions';
import { RootState } from 'src/store';
import { ChrLocation } from '../browserState';
import {
getBrowserNavOpened,
getChrLocation,
getActualChrLocation,
getDefaultChrLocation,
getBrowserActivated,
getGenomeSelectorActive,
getBrowserActiveGenomeId,
getBrowserActiveEnsObject,
isFocusObjectPositionDefault
......@@ -31,21 +29,18 @@ import {
getIsTrackPanelModalOpened,
getIsTrackPanelOpened
} from '../track-panel/trackPanelSelectors';
import { getBreakpointWidth } from 'src/global/globalSelectors';
import { toggleBrowserNav, changeFocusObject } from '../browserActions';
import {
selectTrackPanelTab,
toggleTrackPanel
} from '../track-panel/trackPanelActions';
import { closeDrawer } from '../drawer/drawerActions';
import { RootState } from 'src/store';
import { EnsObject } from 'src/ens-object/ensObjectTypes';
import BrowserReset from '../browser-reset/BrowserReset';
import BrowserGenomeSelector from '../browser-genome-selector/BrowserGenomeSelector';
import TrackPanelTabs from '../track-panel/track-panel-tabs/TrackPanelTabs';
import { getBreakpointWidth } from 'src/global/globalSelectors';
import { BreakpointWidth } from 'src/global/globalConfig';
import styles from './BrowserBar.scss';
export type BrowserBarProps = {
......@@ -59,14 +54,12 @@ export type BrowserBarProps = {
isDrawerOpened: boolean;
isTrackPanelModalOpened: boolean;
isTrackPanelOpened: boolean;
genomeSelectorActive: boolean;
ensObject: EnsObject | null;
selectedTrackPanelTab: TrackSet;
isFocusObjectInDefaultPosition: boolean;
closeDrawer: () => void;
selectTrackPanelTab: (selectedTrackPanelTab: TrackSet) => void;
toggleBrowserNav: () => void;
toggleGenomeSelector: (genomeSelectorActive: boolean) => void;
toggleTrackPanel: (isTrackPanelOpened: boolean) => void;
changeFocusObject: (objectId: string) => void;
};
......@@ -75,25 +68,16 @@ type BrowserInfoProps = {
ensObject: EnsObject;
};
type BrowserNavigatorButtonProps = {
toggleNavigator: () => void;
navigator: BrowserInfoItem;
icon: string; // TODO: use inline SVG
};
export const BrowserBar: FunctionComponent<BrowserBarProps> = (
props: BrowserBarProps
) => {
export const BrowserBar = (props: BrowserBarProps) => {
const { isDrawerOpened } = props;
const shouldShowBrowserInfo = () => {
const { defaultChrLocation } = props;
const isLocationOfWholeChromosome = !defaultChrLocation;