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

Update the browser navigation panel according to the new designs (#205)

* change functionality of the region forms according to new design

* add new breakpoints to the breakpoint getter function

* fix the broken navigation bar layout in smaller screens

* fix styling issues in navigation bar

* fix broken browser navigation bar tests

* remove unused ContentSwitcher import

* add missing test for BrowserNavBarMain

* fix closing track panel  and missing track panel tabs on big desktops

* add breakpoint for big desktop

* fix styling issues with labels in browser nav bar

* fix issues in browser nav bar inputs

* PR suggestions and design changes

* fixed non-unique keys used in track panel bookmarks

* fix issues in nav bar test cases

* more PR suggestions

* fix lint error in browser tests

* change shouldBeOpaque to isGhosted to be more semantic
parent fb419539
Pipeline #51558 passed with stages
in 5 minutes and 1 second
......@@ -16,7 +16,7 @@ jest.mock('./track-panel/TrackPanel', () => () => <div>TrackPanel</div>);
jest.mock('./browser-app-bar/BrowserAppBar', () => () => (
<div>BrowserAppBar</div>
));
jest.mock('ensembl-genome-browser', () => {});
jest.mock('ensembl-genome-browser', () => { return; });
describe('<Browser />', () => {
afterEach(() => {
......
......@@ -105,7 +105,7 @@ export const BrowserBar = (props: BrowserBarProps) => {
const shouldShowTrackPanelTabs =
props.activeGenomeId &&
(props.isTrackPanelOpened ||
props.breakpointWidth === BreakpointWidth.DESKTOP);
props.breakpointWidth >= BreakpointWidth.DESKTOP);
const browserInfoClassName = classNames(styles.browserInfo, {
[styles.browserInfoExpanded]: !props.isTrackPanelOpened,
......@@ -230,7 +230,4 @@ const mapDispatchToProps = {
changeFocusObject
};
export default connect(
mapStateToProps,
mapDispatchToProps
)(BrowserBar);
export default connect(mapStateToProps, mapDispatchToProps)(BrowserBar);
......@@ -25,7 +25,3 @@
top: calc(50% - 20px);
position: absolute;
}
.browserImageOverlay {
z-index: 0;
}
......@@ -31,8 +31,7 @@ describe('<BrowserImage />', () => {
const defaultProps: BrowserImageProps = {
browserCogTrackList: {},
browserNavOpened: false,
regionEditorActive: false,
regionFieldActive: false,
isDisabled: false,
browserActivated: false,
activateBrowser: jest.fn(),
updateBrowserNavStates: jest.fn(),
......@@ -64,14 +63,8 @@ describe('<BrowserImage />', () => {
expect(wrapper.find(ZmenuController).length).toBe(1);
});
test('has an overlay on top when either region field or region editor is active', () => {
const wrapper = mountBrowserImageComponent({ regionFieldActive: true });
expect(wrapper.find(Overlay).length).toBe(1);
wrapper.setProps({
regionFieldActive: false,
regionEditorActive: true
});
test('has an overlay on top when disabled', () => {
const wrapper = mountBrowserImageComponent({ isDisabled: true });
expect(wrapper.find(Overlay).length).toBe(1);
});
});
......
......@@ -38,9 +38,8 @@ import styles from './BrowserImage.scss';
export type BrowserImageProps = {
browserCogTrackList: CogList;
browserNavOpened: boolean;
regionEditorActive: boolean;
regionFieldActive: boolean;
browserActivated: boolean;
isDisabled: boolean;
activateBrowser: () => void;
updateBrowserNavStates: (browserNavStates: BrowserNavStates) => void;
updateBrowserActivated: (browserActivated: boolean) => void;
......@@ -141,8 +140,8 @@ export const BrowserImage = (props: BrowserImageProps) => {
/>
<BrowserCogList />
<ZmenuController browserRef={browserRef} />
{props.regionEditorActive || props.regionFieldActive ? (
<Overlay className={styles.browserImageOverlay} />
{props.isDisabled ? (
<Overlay />
) : null}
</div>
</>
......@@ -153,8 +152,7 @@ const mapStateToProps = (state: RootState) => ({
browserCogTrackList: getBrowserCogTrackList(state),
browserNavOpened: getBrowserNavOpened(state),
browserActivated: getBrowserActivated(state),
regionEditorActive: getRegionEditorActive(state),
regionFieldActive: getRegionFieldActive(state)
isDisabled: getRegionEditorActive(state) || getRegionFieldActive(state)
});
const mapDispatchToProps = {
......
......@@ -13,7 +13,7 @@
label {
font-weight: 300;
margin: 0 5px 0 10px;
margin-right: 5px;
}
input {
......@@ -28,10 +28,7 @@
.browserNavBarButtons {
display: inline-block;
visibility: hidden;
button {
margin-left: 10px;
}
margin-left: 10px;
img {
height: 18px;
......@@ -42,3 +39,7 @@
.browserNavBarButtonsVisible {
visibility: visible;
}
.semiOpaque {
opacity: 0.4;
}
......@@ -2,3 +2,8 @@
display: flex;
align-items: center;
}
.overlay {
background-color: rgba(241, 242, 244, 0.7);
width: 250px;
}
......@@ -7,48 +7,37 @@ import BrowserNavIcon from './BrowserNavIcon';
import { BrowserNavBarControls } from './BrowserNavBarControls';
import { BrowserNavStates } from '../browserState';
import Overlay from 'src/shared/components/overlay/Overlay';
jest.mock('./BrowserNavIcon', () => () => <div>BrowserNavIcon</div>);
const browserNavStates = times(6, faker.random.boolean) as BrowserNavStates;
describe('BrowserNavBarControls', () => {
it('renders without errors', () => {
expect(() =>
mount(
<BrowserNavBarControls
browserNavStates={browserNavStates}
disabled={faker.random.boolean()}
/>
)
).not.toThrow();
});
let wrapper: any;
it('disables all buttons if the disabled prop is set to true', () => {
const wrapper = mount(
beforeEach(() => {
wrapper = mount(
<BrowserNavBarControls
browserNavStates={browserNavStates}
disabled={true}
isDisabled={faker.random.boolean()}
/>
);
const controlButtons = wrapper.find(BrowserNavIcon);
expect(controlButtons.length).toEqual(browserNavStates.length);
controlButtons.forEach((button) => {
expect(button.prop('enabled')).toBe(false);
});
});
it('has an overlay on top when browser nav bar controls are disabled', () => {
wrapper.setProps({ isDisabled: true });
expect(wrapper.find(Overlay).length).toBe(1);
});
it('disables buttons if corresponding actions are not possible', () => {
// browserNavStates are an array of booleans that indicate whether the button
// has already caused maximum corresponding effect, and will have no further effect if pressed
const wrapper = mount(
<BrowserNavBarControls
browserNavStates={browserNavStates}
disabled={false}
/>
);
wrapper.setProps({ isDisabled: false });
const controlButtons = wrapper.find(BrowserNavIcon);
controlButtons.forEach((button, index) => {
controlButtons.forEach((button: any, index: number) => {
const hasCausedMaximumEffect = browserNavStates[index];
expect(button.prop('enabled')).toBe(!hasCausedMaximumEffect);
});
......
......@@ -2,6 +2,7 @@ import React from 'react';
import { connect } from 'react-redux';
import BrowserNavIcon from './BrowserNavIcon';
import Overlay from 'src/shared/components/overlay/Overlay';
import { browserNavConfig, BrowserNavItem } from '../browserConfig';
import {
......@@ -17,36 +18,29 @@ import styles from './BrowserNavBarControls.scss';
type Props = {
browserNavStates: BrowserNavStates;
disabled: boolean;
isDisabled: boolean;
};
export const BrowserNavBarControls = (props: Props) => {
const shouldEnableButton = (index: number) => {
const { browserNavStates, disabled } = props;
const isAtMaximum = browserNavStates[index];
return !disabled && !isAtMaximum;
};
return (
<div className={styles.browserNavBarControls}>
{browserNavConfig.map((item: BrowserNavItem, index: number) => (
<BrowserNavIcon
key={item.name}
browserNavItem={item}
enabled={shouldEnableButton(index)}
/>
))}
</div>
);
};
export const BrowserNavBarControls = (props: Props) => (
<div className={styles.browserNavBarControls}>
{browserNavConfig.map((item: BrowserNavItem, index: number) => (
<BrowserNavIcon
key={item.name}
browserNavItem={item}
enabled={!props.browserNavStates[index]}
/>
))}
{props.isDisabled && <Overlay className={styles.overlay} />}
</div>
);
const mapStateToProps = (state: RootState) => {
const disabled = getRegionEditorActive(state) || getRegionFieldActive(state);
const isDisabled =
getRegionEditorActive(state) || getRegionFieldActive(state);
return {
browserNavStates: getBrowserNavStates(state),
disabled
isDisabled
};
};
......
import React from 'react';
import { mount } from 'enzyme';
import BrowserNavBarMain from './BrowserNavBarMain';
import { BrowserNavBarMain, BrowserNavBarMainProps } from './BrowserNavBarMain';
import ChromosomeNavigator from 'src/content/app/browser/chromosome-navigator/ChromosomeNavigator';
import BrowserNavBarRegionSwitcher from './BrowserNavBarRegionSwitcher';
import { BreakpointWidth } from 'src/global/globalConfig';
jest.mock(
'src/content/app/browser/chromosome-navigator/ChromosomeNavigator',
......@@ -14,15 +15,35 @@ jest.mock('./BrowserNavBarRegionSwitcher', () => () => (
<div>BrowserNavBarRegionSwitcher</div>
));
const props: BrowserNavBarMainProps = {
viewportWidth: BreakpointWidth.TABLET
};
describe('BrowserNavBarMain', () => {
it('renders chromosome visualization by default', () => {
const wrapper = mount(<BrowserNavBarMain />);
let wrapper: any;
beforeEach(() => {
wrapper = mount(<BrowserNavBarMain {...props} />);
});
afterEach(() => {
jest.resetAllMocks();
});
it('does not render chromosome visualization by default for screens smaller than laptops', () => {
expect(wrapper.find(ChromosomeNavigator).length).toBe(0);
});
it('renders chromosome visualization by default for laptops or bigger screens', () => {
wrapper.setProps({ viewportWidth: BreakpointWidth.LAPTOP });
expect(wrapper.find(ChromosomeNavigator).length).toBe(1);
expect(wrapper.find(BrowserNavBarRegionSwitcher).length).toBe(0);
});
it('renders RegionSwitcher when user clicks on Change', () => {
const wrapper = mount(<BrowserNavBarMain />);
wrapper.setProps({ viewportWidth: BreakpointWidth.LAPTOP });
const changeButton = wrapper.find('.contentSwitcher');
changeButton.simulate('click');
expect(wrapper.find(ChromosomeNavigator).length).toBe(0);
......@@ -30,7 +51,8 @@ describe('BrowserNavBarMain', () => {
});
it('renders chromosome visualization when user closes RegionSwitcher', () => {
const wrapper = mount(<BrowserNavBarMain />);
wrapper.setProps({ viewportWidth: BreakpointWidth.LAPTOP });
const changeButton = wrapper.find('.contentSwitcher');
changeButton.simulate('click');
......
import React, { useState } from 'react';
import { connect } from 'react-redux';
import classNames from 'classnames';
import { RootState } from 'src/store';
import { getBreakpointWidth } from 'src/global/globalSelectors';
import { BreakpointWidth } from 'src/global/globalConfig';
import ChromosomeNavigator from 'src/content/app/browser/chromosome-navigator/ChromosomeNavigator';
import BrowserNavBarRegionSwitcher from './BrowserNavBarRegionSwitcher';
import { ReactComponent as CloseIcon } from 'static/img/shared/close.svg';
import styles from './BrowserNavBarMain.scss';
......@@ -12,17 +18,25 @@ enum Content {
REGION_SWITCHER
}
const BrowserNavBarMain = () => {
export type BrowserNavBarMainProps = {
viewportWidth: BreakpointWidth;
};
export const BrowserNavBarMain = (props: BrowserNavBarMainProps) => {
const [view, changeView] = useState<Content>(Content.CHROMOSOME);
const handleViewChange = (newView: Content) => {
changeView(newView);
};
const shouldShowChromosomeNavigator =
props.viewportWidth >= BreakpointWidth.LAPTOP &&
view === Content.CHROMOSOME;
return (
<div className={styles.browserNavBarMain}>
<div className={styles.content}>
{view === Content.CHROMOSOME ? (
{shouldShowChromosomeNavigator ? (
<div className={styles.contentChromosomeNavigator}>
<ChromosomeNavigator />
</div>
......@@ -30,7 +44,9 @@ const BrowserNavBarMain = () => {
<BrowserNavBarRegionSwitcher />
)}
</div>
<ContentSwitcher currentView={view} onSwitch={handleViewChange} />
{props.viewportWidth >= BreakpointWidth.LAPTOP && (
<ContentSwitcher currentView={view} onSwitch={handleViewChange} />
)}
</div>
);
};
......@@ -65,4 +81,8 @@ const ContentSwitcher = (props: ContentSwitcherProps) => {
);
};
export default BrowserNavBarMain;
const mapStateToProps = (state: RootState) => ({
viewportWidth: getBreakpointWidth(state)
});
export default connect(mapStateToProps)(BrowserNavBarMain);
......@@ -2,10 +2,11 @@ import React from 'react';
import { mount } from 'enzyme';
import { BrowserNavBarRegionSwitcher } from './BrowserNavBarRegionSwitcher';
import BrowserRegionEditor from '../browser-region-editor/BrowserRegionEditor';
import BrowserRegionField from '../browser-region-field/BrowserRegionField';
import { BreakpointWidth } from 'src/global/globalConfig';
jest.mock(
'src/content/app/browser/browser-region-editor/BrowserRegionEditor',
() => () => <div>BrowserRegionEditor</div>
......@@ -16,20 +17,34 @@ jest.mock(
);
const props = {
viewportWidth: BreakpointWidth.TABLET,
toggleRegionEditorActive: jest.fn(),
toggleRegionFieldActive: jest.fn()
};
describe('BrowserNavBarRegionSwitcher', () => {
let wrapper: any;
beforeEach(() => {
wrapper = mount(<BrowserNavBarRegionSwitcher {...props} />);
});
afterEach(() => {
jest.resetAllMocks();
});
it('renders correctly', () => {
const wrapper = mount(<BrowserNavBarRegionSwitcher {...props} />);
describe('rendering', () => {
it('renders region field and not region editor on smaller screens', () => {
expect(wrapper.find(BrowserRegionField)).toHaveLength(1);
expect(wrapper.find(BrowserRegionEditor)).toHaveLength(0);
});
it('renders both region field and region editor on big desktop screens', () => {
wrapper.setProps({ viewportWidth: BreakpointWidth.BIG_DESKTOP });
expect(wrapper.find(BrowserRegionEditor).length).toBe(1);
expect(wrapper.find(BrowserRegionField).length).toBe(1);
expect(wrapper.find(BrowserRegionEditor)).toHaveLength(1);
expect(wrapper.find(BrowserRegionField)).toHaveLength(1);
});
});
it('calls cleanup functions on unmount', () => {
......
......@@ -4,14 +4,20 @@ import { connect } from 'react-redux';
import BrowserRegionEditor from '../browser-region-editor/BrowserRegionEditor';
import BrowserRegionField from '../browser-region-field/BrowserRegionField';
import { BreakpointWidth } from 'src/global/globalConfig';
import {
toggleRegionEditorActive,
toggleRegionFieldActive
} from '../browserActions';
import { getBreakpointWidth } from 'src/global/globalSelectors';
import { RootState } from 'src/store';
import styles from './BrowserNavBarRegionSwitcher.scss';
type Props = {
viewportWidth: BreakpointWidth;
toggleRegionEditorActive: (isActive: boolean) => void;
toggleRegionFieldActive: (isActive: boolean) => void;
};
......@@ -31,19 +37,25 @@ export const BrowserNavBarRegionSwitcher = (props: Props) => {
<div className={styles.regionFieldWrapper}>
<BrowserRegionField />
</div>
<div className={styles.regionEditorWrapper}>
<BrowserRegionEditor />
</div>
{props.viewportWidth >= BreakpointWidth.BIG_DESKTOP && (
<div className={styles.regionEditorWrapper}>
<BrowserRegionEditor />
</div>
)}
</div>
);
};
const mapStateToProps = (state: RootState) => ({
viewportWidth: getBreakpointWidth(state)
});
const mapDispatchToProps = {
toggleRegionEditorActive,
toggleRegionFieldActive
};
export default connect(
null,
mapStateToProps,
mapDispatchToProps
)(BrowserNavBarRegionSwitcher);
......@@ -2,7 +2,7 @@
.browserNavIcon {
display: inline-block;
margin: 0 20px 0 0;
margin: 0 13px 0 0;
& > img {
height: 22px;
......
......@@ -3,6 +3,10 @@
.browserRegionEditor {
position: relative;
label {
margin-left: 10px;
}
input {
width: 120px;
}
......@@ -12,3 +16,7 @@
position: relative;
}
}
.overlay {
background-color: rgba(241, 242, 244, 0.7);
}
......@@ -19,10 +19,6 @@ import {
} from 'tests/fixtures/browser';
import * as browserHelper from '../browserHelper';
jest.mock('src/shared/components/overlay/Overlay', () => () => (
<div>Overlay</div>
));
describe('<BrowserRegionEditor', () => {
afterEach(() => {
jest.resetAllMocks();
......@@ -57,10 +53,9 @@ describe('<BrowserRegionEditor', () => {
test('contains submit and close buttons', () => {
expect(wrapper.find('button[type="submit"]')).toHaveLength(1);
expect(wrapper.find('button[role="closeButton"]')).toHaveLength(1);
});
test('has an overlay on top when region field is active', () => {
test('has an overlay on top when disabled', () => {
wrapper.setProps({ isDisabled: true });
expect(wrapper.find(Overlay).length).toBe(1);
});
......@@ -68,7 +63,7 @@ describe('<BrowserRegionEditor', () => {
describe('behaviour', () => {
test('shows form buttons when focussed', () => {
wrapper.find(Select).simulate('focus');
wrapper.find('form').simulate('focus');
expect(wrapper.props().toggleRegionEditorActive).toHaveBeenCalledTimes(1);
});
......@@ -104,41 +99,7 @@ describe('<BrowserRegionEditor', () => {
});
});
test('resets region editor form when close button is clicked', () => {
const [, locationStart, locationEnd] = wrapper.props().chrLocation;
const locationStartInput = getCommaSeparatedNumber(faker.random.number());
const locationEndInput = getCommaSeparatedNumber(faker.random.number());
wrapper
.find(Input)
.first()
.simulate('change', { target: { value: locationStartInput } });
wrapper
.find(Input)
.last()
.simulate('change', {
target: { value: locationEndInput }
});
wrapper.find