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: persist pane sizes and fix resizing bug #15480

Merged
merged 5 commits into from
Mar 15, 2021
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
49 changes: 37 additions & 12 deletions packages/runner-ct/cypress/component/RunnerCt.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,27 @@ import '@packages/runner/src/main.scss'

const selectors = {
reporter: '[data-cy=reporter]',
noSpecSelectedReporter: '[data-cy=no-spec-selected-reporter]',
specsList: '[data-cy=specs-list]',
searchInput: 'input[placeholder="Find spec..."]',
}

interface Overrides {
saveState?: Function
}

const noop = () => {}

class FakeEventManager {
start = () => { }
on = () => { }
stop = () => {}
notifyRunningSpec = () => { }
constructor (overrides: Overrides = {}) {
this.saveState = overrides.saveState || noop
}

start = noop
on = noop
stop = noop
notifyRunningSpec = noop
saveState: Function = () => { }
}

const fakeConfig = { projectName: 'Project', env: {}, isTextTerminal: false } as any as Cypress.RuntimeConfigOptions
Expand Down Expand Up @@ -58,30 +70,43 @@ describe('RunnerCt', () => {
})

context('keyboard shortcuts', () => {
beforeEach(() => {
it('toggles specs list drawer using shortcut', () => {
const saveState = cy.stub()

mount(
<RunnerCt
state={makeState()}
// @ts-ignore - this is difficult to stub. Real one breaks things.
eventManager={new FakeEventManager()}
eventManager={new FakeEventManager({ saveState })}
config={fakeConfig}
/>,
)

cy.window().then((win) => win.focus())
})

it('toggles specs list drawer using shortcut', () => {
cy.get(selectors.specsList).should('be.visible')

cy.realPress(['Meta', 'B'])
cy.get(selectors.specsList).should('not.be.visible')
cy.get(selectors.specsList).should('not.be.visible').then(() => {
expect(saveState).to.have.been.calledWith({ ctIsSpecsListOpen: false })
})

cy.realPress(['Meta', 'B'])
cy.get(selectors.specsList).should('be.visible')
cy.get(selectors.specsList).should('be.visible').then(() => {
expect(saveState).to.have.been.calledWith({ ctIsSpecsListOpen: false }),
expect(saveState).to.have.been.calledWith({ ctIsSpecsListOpen: true })
})
})

it('focuses the search field on "/"', () => {
mount(
<RunnerCt
state={makeState()}
// @ts-ignore - this is difficult to stub. Real one breaks things.
eventManager={new FakeEventManager()}
config={fakeConfig}
/>,
)

cy.realPress('/')
cy.get(selectors.searchInput).should('be.focused')
})
Expand All @@ -95,7 +120,7 @@ describe('RunnerCt', () => {
eventManager={new FakeEventManager()}
config={fakeConfig} />)

cy.get(selectors.reporter).should('not.be.visible')
cy.get(selectors.noSpecSelectedReporter).should('exist')
cy.percySnapshot()
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/runner-ct/src/app/ReporterContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const ReporterContainer = observer(
function ReporterContainer (props: ReporterContainerProps) {
if (!props.state.spec) {
return (
<div className='no-spec' data-cy="reporter">
<div className='no-spec' data-cy="no-spec-selected-reporter">
<NoSpecSelected />
</div>
)
Expand Down
56 changes: 37 additions & 19 deletions packages/runner-ct/src/app/RunnerCt.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ const App: React.FC<AppProps> = observer(
}, [])

React.useEffect(() => {
const isOpenMode = !config.isTextTerminal

state.setIsSpecsListOpen(isOpenMode)
if (config.isTextTerminal) {
state.setIsSpecsListOpen(false)
}
}, [])

useScreenshotHandler({
Expand All @@ -124,7 +124,10 @@ const App: React.FC<AppProps> = observer(
type: 'js',
onClick: ({ index }) => {
onNavItemClick(index)
state.setIsSpecsListOpen(!props.state.isSpecsListOpen)
const isOpen = !props.state.isSpecsListOpen

state.setIsSpecsListOpen(isOpen)
props.eventManager.saveState({ ctIsSpecsListOpen: isOpen })
},
},
},
Expand All @@ -150,7 +153,10 @@ const App: React.FC<AppProps> = observer(

function toggleSpecsList () {
setActiveIndex((val) => val === 0 ? undefined : 0)
state.setIsSpecsListOpen(!props.state.isSpecsListOpen)
const newVal = !props.state.isSpecsListOpen

state.setIsSpecsListOpen(newVal)
props.eventManager.saveState({ ctIsSpecsListOpen: newVal })
}

function focusSpecsList () {
Expand Down Expand Up @@ -179,6 +185,12 @@ const App: React.FC<AppProps> = observer(
state.updateSpecListWidth(newWidth)
}

function persistWidth (prop: 'ctReporterWidth' | 'ctSpecListWidth') {
return (newWidth: number) => {
props.eventManager.saveState({ [prop]: newWidth })
}
}

function hideIfScreenshotting (callback: () => number) {
if (state.screenshotting) {
return 0
Expand Down Expand Up @@ -218,6 +230,21 @@ const App: React.FC<AppProps> = observer(
<KeyboardHelper />
)

const MainAreaComponent: React.FC | typeof SplitPane = props.state.spec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the second bug described in the PR description. We should not render the Reporter at all if there is no spec selected. It makes resizing buggy.

? SplitPane
: (props) => <div>{props.children}</div>

const mainAreaProps = props.state.spec
? {
split: 'vertical',
minSize: hideReporterIfNecessary(() => 100),
maxSize: hideReporterIfNecessary(() => 600),
defaultSize: hideReporterIfNecessary(() => state.reporterWidth),
className: 'primary',
onChange: debounce(onReporterSplitPaneChange),
}
: {}

return (
<SplitPane
split="vertical"
Expand All @@ -229,10 +256,10 @@ const App: React.FC<AppProps> = observer(
{leftNav}
<SplitPane
split="vertical"
// do not allow resizing of this for now, simplifes calculation for scale of AUT.
minSize={hideIfScreenshotting(() => state.isSpecsListOpen ? 30 : 0)}
maxSize={hideIfScreenshotting(() => state.isSpecsListOpen ? 600 : 0)}
defaultSize={hideIfScreenshotting(() => state.isSpecsListOpen ? DEFAULT_LIST_WIDTH : 0)}
defaultSize={hideIfScreenshotting(() => state.isSpecsListOpen ? state.specListWidth : 0)}
onDragFinished={persistWidth('ctSpecListWidth')}
className="primary"
// @ts-expect-error split-pane ref types are weak so we are using our custom type for ref
ref={splitPaneRef}
Expand All @@ -250,15 +277,7 @@ const App: React.FC<AppProps> = observer(
}
onSelectSpec={runSpec}
/>

<SplitPane
split="vertical"
minSize={hideReporterIfNecessary(() => 100)}
maxSize={hideReporterIfNecessary(() => 600)}
defaultSize={hideReporterIfNecessary(() => DEFAULT_REPORTER_WIDTH)}
className="primary"
onChange={debounce(onReporterSplitPaneChange)}
>
<MainAreaComponent {...mainAreaProps}>
<ReporterContainer
state={props.state}
config={props.config}
Expand All @@ -271,7 +290,7 @@ const App: React.FC<AppProps> = observer(
allowResize={props.state.isAnyDevtoolsPluginOpen}
size={hideIfScreenshotting(() =>
state.isAnyDevtoolsPluginOpen
? DEFAULT_PLUGINS_HEIGHT
? state.pluginsHeight
// show the small not resize-able panel with buttons or nothing
: state.isAnyPluginToShow ? PLUGIN_BAR_HEIGHT : 0)}
onChange={debounce(onPluginsSplitPaneChange)}
Expand All @@ -296,9 +315,8 @@ const App: React.FC<AppProps> = observer(
pluginRootContainer={pluginRootContainer}
/>
</SplitPane>
</SplitPane>
</MainAreaComponent>
</SplitPane>

</SplitPane>
)
},
Expand Down
3 changes: 2 additions & 1 deletion packages/runner-ct/src/lib/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,10 @@ export default class State {
multiSpecs = [],
reporterWidth = DEFAULT_REPORTER_WIDTH,
specListWidth = DEFAULT_LIST_WIDTH,
isSpecsListOpen = true,
}) {
this.reporterWidth = reporterWidth
this.pluginsHeight = PLUGIN_BAR_HEIGHT
this.isSpecsListOpen = isSpecsListOpen
this.spec = spec
this.specs = specs
this.specListWidth = specListWidth
Expand Down
7 changes: 6 additions & 1 deletion packages/runner-ct/src/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ const Runner = {

configState.specs = config.specs

const state = new State(configState)
const ctRunnerSpecificDefaults = {
reporterWidth: config.state.ctReporterWidth,
isSpecsListOpen: config.state.ctIsSpecsListOpen,
specListWidth: config.state.ctSpecListWidth,
}
const state = new State({ ...configState, ...ctRunnerSpecificDefaults })

const setSpecByUrlHash = () => {
const specPath = util.specPath()
Expand Down
3 changes: 3 additions & 0 deletions packages/server/lib/saved_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ reporterWidth
showedOnBoardingModal
showedStudioModal
preferredOpener
ctReporterWidth
ctIsSpecsListOpen
ctSpecListWidth
`.trim().split(/\s+/)

const formStatePath = (projectRoot) => {
Expand Down