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

[WIP] toggle project text search#3266

Merged
jasonLaster merged 2 commits into
firefox-devtools:masterfrom
jasonLaster:toggle-psearch
Jul 6, 2017
Merged

[WIP] toggle project text search#3266
jasonLaster merged 2 commits into
firefox-devtools:masterfrom
jasonLaster:toggle-psearch

Conversation

@jasonLaster
Copy link
Copy Markdown
Contributor

@jasonLaster jasonLaster commented Jul 5, 2017

Associated Issue: #3091

Summary of Changes

Right now we map cmd+p to project text search if it is enabled. We should be able to have cmd+p map to source search and cmd+shift+f map to project text search.

This does that. It doesn't add the bottom bar for toggling, but it helps.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 5, 2017

Codecov Report

Merging #3266 into master will decrease coverage by 0.23%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3266      +/-   ##
==========================================
- Coverage    49.4%   49.17%   -0.24%     
==========================================
  Files          99       99              
  Lines        4151     4140      -11     
  Branches      855      854       -1     
==========================================
- Hits         2051     2036      -15     
- Misses       2100     2104       +4
Impacted Files Coverage Δ
src/reducers/ui.js 18.57% <ø> (ø) ⬆️
src/actions/ui.js 5.88% <0%> (ø) ⬆️
src/utils/source.js 46.15% <0%> (-2.87%) ⬇️
src/components/Editor/SearchBar.js 21.08% <0%> (ø) ⬆️
src/components/Editor/SymbolModal.js 31.45% <0%> (ø) ⬆️
src/utils/parser/getOutOfScopeLocations.js 84% <0%> (-8.6%) ⬇️
src/utils/parser/utils/ast.js 80.64% <0%> (-0.61%) ⬇️
src/utils/parser/getSymbols.js 96.15% <0%> (+0.37%) ⬆️
src/components/Editor/index.js 18.94% <0%> (+0.4%) ⬆️

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 e3f6a07...3a9321c. Read the comment docs.

@jasonLaster jasonLaster force-pushed the toggle-psearch branch 2 times, most recently from be64944 to 34cb6ba Compare July 5, 2017 22:02
Copy link
Copy Markdown
Contributor Author

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

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

some thoughts

highlightLineRange: ({ start: number, end: number }) => void,
clearHighlightLineRange: () => void,
searchOn?: boolean,
toggleActiveSearch: (?ActiveSearchType) => any,
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.

switching from the verb toggle to set because we have more than two now: Project, source, file, symbol

import { connect } from "react-redux";
import { bindActionCreators } from "redux";
import actions from "../../actions";
import { getSources, getActiveSearchState } from "../../selectors";
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.

moving source search into its own component will untangle a lot of this code

id: `${result.text}/${result.line}/${result.column}`
}))
);
}
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.

so... this wasn't used :)

import { endTruncateStr } from "../../utils/utils";
import { parse as parseURL } from "url";
import { isPretty } from "../../utils/source";
import { isEnabled } from "devtools-config";
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.

ProjectSearch now is a coordinator, it just decides which search UI to show and will eventually have buttons to switch between them

Copy link
Copy Markdown
Contributor

@codehag codehag left a comment

Choose a reason for hiding this comment

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

Looks really good so far, let me know if you want me to look at it again

@jasonLaster jasonLaster merged commit 2dd5e69 into firefox-devtools:master Jul 6, 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