Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Bug/2963 search shows no results after switching to another tool and switching back to debugger#3121

Merged
jasonLaster merged 8 commits into
firefox-devtools:masterfrom
codehag:bug/2963-search-shows-no-results-after-switching-to-another-tool-and-switching-back-to-debugger
Jun 7, 2017
Merged

Bug/2963 search shows no results after switching to another tool and switching back to debugger#3121
jasonLaster merged 8 commits into
firefox-devtools:masterfrom
codehag:bug/2963-search-shows-no-results-after-switching-to-another-tool-and-switching-back-to-debugger

Conversation

@codehag

@codehag codehag commented Jun 7, 2017

Copy link
Copy Markdown
Contributor

Associated Issue: #2963

Comment thread src/components/Editor/SearchBar.js Outdated
const { query, symbolSearchOn } = this.props;
const { selectedResultIndex } = this.state;
const { query, symbolSearchResults, symbolSearchOn } = this.props;
console.log("hi");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👋

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ack

Comment thread src/reducers/tests/ui.js Outdated
const state = State();
expect(getSymbolSearchResults({ ui: state })).toBe(
state.symbolSearchResults
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like treating actions as the public interface here:

here's an actions/tests/ui test

  it("should toggle the visible state of project search", () => {
    const { dispatch, getState } = createStore();
    expect(getProjectSearchState(getState())).toBe(false);
    dispatch(actions.toggleProjectSearch());
    expect(getProjectSearchState(getState())).toBe(true);
  });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess false / true should be replaced with something else? or maybe we drop these tests for now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh, i think this is a good "model" test

@codecov

codecov Bot commented Jun 7, 2017

Copy link
Copy Markdown

Codecov Report

Merging #3121 into master will decrease coverage by 17.07%.
The diff coverage is 52.38%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3121       +/-   ##
===========================================
- Coverage   66.86%   49.78%   -17.08%     
===========================================
  Files          76       95       +19     
  Lines        2698     3993     +1295     
  Branches      544      815      +271     
===========================================
+ Hits         1804     1988      +184     
- Misses        894     2005     +1111
Impacted Files Coverage Δ
src/selectors.js 100% <ø> (ø) ⬆️
src/components/Editor/index.js 20.43% <ø> (ø)
src/actions/ui.js 77.5% <0%> (-8.62%) ⬇️
src/reducers/ui.js 82.19% <50%> (-3.97%) ⬇️
src/components/Editor/SearchBar.js 22.43% <77.77%> (ø)
src/utils/parser/index.js 100% <0%> (ø) ⬆️
src/components/shared/ManagedTree.js 5.35% <0%> (ø)
src/components/Editor/ConditionalPanel.js 0% <0%> (ø)
src/components/Editor/Preview.js 5.17% <0%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad7986c...0faa2be. Read the comment docs.

@codecov

codecov Bot commented Jun 7, 2017

Copy link
Copy Markdown

Codecov Report

Merging #3121 into master will decrease coverage by 18.65%.
The diff coverage is 54.54%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3121       +/-   ##
===========================================
- Coverage   66.28%   47.63%   -18.66%     
===========================================
  Files          76       95       +19     
  Lines        2714     4012     +1298     
  Branches      549      821      +272     
===========================================
+ Hits         1799     1911      +112     
- Misses        915     2101     +1186
Impacted Files Coverage Δ
src/components/Editor/index.js 20.32% <ø> (ø)
src/selectors.js 100% <ø> (ø) ⬆️
src/reducers/ui.js 24.65% <50%> (-61.5%) ⬇️
src/actions/ui.js 5% <50%> (-81.12%) ⬇️
src/components/Editor/SearchBar.js 22.11% <60%> (ø)
src/components/Editor/Footer.js 3.84% <0%> (ø)
src/components/Editor/ConditionalPanel.js 0% <0%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1278f11...727ab03. Read the comment docs.

@jasonLaster jasonLaster force-pushed the bug/2963-search-shows-no-results-after-switching-to-another-tool-and-switching-back-to-debugger branch from 333973f to 727ab03 Compare June 7, 2017 22:40
@jasonLaster jasonLaster merged commit d61bb3e into firefox-devtools:master Jun 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants