Skip to content

Commit

Permalink
fix: render query history panel only when it's toggled (#1821)
Browse files Browse the repository at this point in the history
Use application to hide query history panel instead of CSS. Improves perf, reduces regression surface area.

Co-authored-by: Rikki Schulte <[email protected]>
  • Loading branch information
harshithpabbati and acao authored Apr 4, 2021
1 parent 2d91916 commit 9f8c78c
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-cougars-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'graphiql': patch
---

fix: render query history panel only when it's toggled, instead of hiding with CSS
2 changes: 2 additions & 0 deletions .github/workflows/deploy-preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- '**.md'
- 'examples'
- '!examples/monaco-graphql-webpack'
pull_request:
types: [opened, synchronize]

jobs:
build:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/license-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ on:
push:
paths:
- 'yarn.lock'
pull_request:
types: [opened, synchronize]

jobs:
check:
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/push-pr.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
name: CI
on: [push]
on:
pull_request:
types: [opened, synchronize]

jobs:
lint:
Expand Down
40 changes: 21 additions & 19 deletions packages/graphiql/src/components/GraphiQL.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -482,25 +482,27 @@ export class GraphiQL extends React.Component<GraphiQLProps, GraphiQLState> {
this.graphiqlContainer = n;
}}
className="graphiql-container">
<div className="historyPaneWrap" style={historyPaneStyle}>
<QueryHistory
ref={node => {
this._queryHistory = node;
}}
operationName={this.state.operationName}
query={this.state.query}
variables={this.state.variables}
onSelectQuery={this.handleSelectHistoryQuery}
storage={this._storage}
queryID={this._editorQueryID}>
<button
className="docExplorerHide"
onClick={this.handleToggleHistory}
aria-label="Close History">
{'\u2715'}
</button>
</QueryHistory>
</div>
{this.state.historyPaneOpen && (
<div className="historyPaneWrap" style={historyPaneStyle}>
<QueryHistory
ref={node => {
this._queryHistory = node;
}}
operationName={this.state.operationName}
query={this.state.query}
variables={this.state.variables}
onSelectQuery={this.handleSelectHistoryQuery}
storage={this._storage}
queryID={this._editorQueryID}>
<button
className="docExplorerHide"
onClick={this.handleToggleHistory}
aria-label="Close History">
{'\u2715'}
</button>
</QueryHistory>
</div>
)}
<div className="editorWrap">
<div className="topBarWrap">
<div className="topBar">
Expand Down
12 changes: 12 additions & 0 deletions packages/graphiql/src/components/__tests__/GraphiQL.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ describe('GraphiQL', () => {
expect(queryVariables3?.style.height).toEqual('');
});

it('defaults to closed history panel', () => {
const { container } = render(<GraphiQL fetcher={noOpFetcher} />);
expect(container.querySelector('.historyPaneWrap')).not.toBeInTheDocument();
});

it('adds a history item when the execute query function button is clicked', () => {
const { getByTitle, container } = render(
<GraphiQL
Expand All @@ -180,6 +185,7 @@ describe('GraphiQL', () => {
fetcher={noOpFetcher}
/>,
);
fireEvent.click(getByTitle('Show History'));
fireEvent.click(getByTitle('Execute Query (Ctrl-Enter)'));
expect(container.querySelectorAll('.history-contents li')).toHaveLength(1);
});
Expand All @@ -188,6 +194,7 @@ describe('GraphiQL', () => {
const { getByTitle, container } = render(
<GraphiQL query={mockBadQuery} fetcher={noOpFetcher} />,
);
fireEvent.click(getByTitle('Show History'));
fireEvent.click(getByTitle('Execute Query (Ctrl-Enter)'));
expect(container.querySelectorAll('.history-contents li')).toHaveLength(0);
});
Expand All @@ -202,6 +209,7 @@ describe('GraphiQL', () => {
headers={mockHeaders1}
/>,
);
fireEvent.click(getByTitle('Show History'));
fireEvent.click(getByTitle('Execute Query (Ctrl-Enter)'));
expect(container.querySelectorAll('.history-contents li')).toHaveLength(1);
});
Expand All @@ -216,6 +224,7 @@ describe('GraphiQL', () => {
headers={mockHeaders1}
/>,
);
fireEvent.click(getByTitle('Show History'));
fireEvent.click(getByTitle('Execute Query (Ctrl-Enter)'));
expect(container.querySelectorAll('.history-contents li')).toHaveLength(1);
fireEvent.click(getByTitle('Execute Query (Ctrl-Enter)'));
Expand All @@ -232,6 +241,7 @@ describe('GraphiQL', () => {
headers={mockHeaders1}
/>,
);
fireEvent.click(getByTitle('Show History'));
const executeQueryButton = getByTitle('Execute Query (Ctrl-Enter)');
fireEvent.click(executeQueryButton);
expect(container.querySelectorAll('.history-contents li')).toHaveLength(1);
Expand Down Expand Up @@ -260,6 +270,7 @@ describe('GraphiQL', () => {
headers={mockHeaders1}
/>,
);
fireEvent.click(getByTitle('Show History'));
const executeQueryButton = getByTitle('Execute Query (Ctrl-Enter)');
fireEvent.click(executeQueryButton);
expect(container.querySelectorAll('.history-label')).toHaveLength(1);
Expand All @@ -286,6 +297,7 @@ describe('GraphiQL', () => {
headerEditorEnabled
/>,
);
fireEvent.click(getByTitle('Show History'));
const executeQueryButton = getByTitle('Execute Query (Ctrl-Enter)');
fireEvent.click(executeQueryButton);
expect(container.querySelectorAll('.history-label')).toHaveLength(1);
Expand Down

1 comment on commit 9f8c78c

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.