From 6b652e8f01e6c052f36c5441ee038bea5b4438b5 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Mon, 21 Feb 2022 16:58:47 +0100 Subject: [PATCH] Do not pop if there is nothing to pop --- src/components/shared/FilterNavigatorBar.js | 44 ++++++++++++----- .../components/FilterNavigatorBar.test.js | 48 ++++++++++++++++++- .../FilterNavigatorBar.test.js.snap | 11 ++--- .../__snapshots__/FlameGraph.test.js.snap | 1 - .../ProfileCallTreeView.test.js.snap | 21 -------- .../__snapshots__/StackChart.test.js.snap | 3 -- 6 files changed, 81 insertions(+), 47 deletions(-) diff --git a/src/components/shared/FilterNavigatorBar.js b/src/components/shared/FilterNavigatorBar.js index 563c44e475..0fe2bb0257 100644 --- a/src/components/shared/FilterNavigatorBar.js +++ b/src/components/shared/FilterNavigatorBar.js @@ -9,23 +9,43 @@ import classNames from 'classnames'; import { CSSTransition, TransitionGroup } from 'react-transition-group'; import './FilterNavigatorBar.css'; +type FilterNavigatorBarButtonProps = {| + onClick: (number) => mixed, + index: number, + children: React.Node, +|}; + +class FilterNavigatorBarButton extends React.PureComponent { + _onClick = () => { + const { index, onClick } = this.props; + onClick(index); + }; + + render() { + return ( + + ); + } +} + type Props = {| +className: string, - +items: $ReadOnlyArray, + +items: $ReadOnlyArray, +onPop: (number) => mixed, +selectedItem: number, +uncommittedItem?: string, |}; export class FilterNavigatorBar extends React.PureComponent { - _onLiClick = (e: SyntheticMouseEvent) => { - const element = e.currentTarget; - const index = parseInt(element.dataset.index, 10) || 0; - this.props.onPop(index); - }; - render() { - const { className, items, selectedItem, uncommittedItem } = this.props; + const { className, items, selectedItem, uncommittedItem, onPop } = + this.props; return ( { timeout={250} >
  • - {i === items.length - 1 ? ( + {i === items.length - 1 && !uncommittedItem ? ( {item} ) : ( - + )}
    • diff --git a/src/test/components/FilterNavigatorBar.test.js b/src/test/components/FilterNavigatorBar.test.js index 662d4ab38f..1f9c399f15 100644 --- a/src/test/components/FilterNavigatorBar.test.js +++ b/src/test/components/FilterNavigatorBar.test.js @@ -6,13 +6,59 @@ import * as React from 'react'; import { Provider } from 'react-redux'; -import { render } from 'firefox-profiler/test/fixtures/testing-library'; +import { + render, + fireEvent, + screen, +} from 'firefox-profiler/test/fixtures/testing-library'; +import { FilterNavigatorBar } from 'firefox-profiler/components/shared/FilterNavigatorBar'; import { ProfileFilterNavigator } from '../../components/app/ProfileFilterNavigator'; import * as ProfileView from '../../actions/profile-view'; import * as ReceiveProfile from '../../actions/receive-profile'; import { storeWithProfile } from '../fixtures/stores'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; +describe('shared/FilterNavigatorBar', () => { + it(`pops the item unless the last one is clicked`, () => { + const onPop = jest.fn(); + render( + + ); + + // We don't use getByRole because this isn't a button. + const lastElement = screen.getByText('bar'); + fireEvent.click(lastElement); + expect(onPop).not.toHaveBeenCalled(); + + const firstElement = screen.getByRole('button', { name: 'foo' }); + fireEvent.click(firstElement); + expect(onPop).toHaveBeenCalledWith(0); + }); + + it(`pops the last item if there's an uncommited item`, () => { + const onPop = jest.fn(); + render( + + ); + + // We don't use getByRole because this isn't a button. + const lastElement = screen.getByText('bar'); + fireEvent.click(lastElement); + expect(onPop).toHaveBeenCalledWith(1); + }); +}); + describe('app/ProfileFilterNavigator', () => { const tabID = 123123; function setup() { diff --git a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap index efaebbd70c..13cc7bbd05 100644 --- a/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap +++ b/src/test/components/__snapshots__/FilterNavigatorBar.test.js.snap @@ -6,7 +6,6 @@ exports[`app/ProfileFilterNavigator renders ProfileFilterNavigator properly 1`] >