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

Outline matches #1091

Merged
merged 6 commits into from Nov 4, 2016
Merged

Outline matches #1091

merged 6 commits into from Nov 4, 2016

Conversation

bomsy
Copy link
Contributor

@bomsy bomsy commented Nov 3, 2016

Associated Issue: #800

Summary of Changes

  • Added new search overlay for outlining matches
  • Added css for styling outlines

Testing

  • passes npm test
  • passes npm run lint

Screenshots/Videos

outlines

@bomsy
Copy link
Contributor Author

bomsy commented Nov 3, 2016

Was not sure about the theme color variable to use for selections.

@@ -32,6 +33,31 @@ function getSearchCursor(cm, query, pos) {
typeof query == "string" && query == query.toLowerCase());
}

function searchOverlay(query) {
query = query.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
Copy link
Contributor

Choose a reason for hiding this comment

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

lol. this PR is crazy town, do you have some context on how you came up with it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had a look into an example of how code mirror does highlighting of text. Basically Codemirror's addOverlay function takes a mode object. The mode object should have a token function which is applied as each token in the file is parsed. https://codemirror.net/doc/manual.html#option_mode

The regex just escapes any special characters in the input

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should i add as comments to the code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - i think that would help

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool .. will do!

@@ -125,6 +125,14 @@ selector in floating-scrollbar-light.css across all platforms. */
background-repeat: repeat-x;
}

.cm-selecting {
background: none;
border-width: 1px;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a minor thing, but would you mind trying outline as well. I notice that border pushes the text, which is a slight visual tick that atom and i believe other editors avoid

Copy link
Contributor

@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.

Looks great!

I spoke with Helen today and she'll share some colors as a follow up improvement.

Merging, but out of curiosity could we replace the regex escape with lodashes escape fnc.

@jasonLaster jasonLaster merged commit 45fd789 into firefox-devtools:master Nov 4, 2016
@bomsy
Copy link
Contributor Author

bomsy commented Nov 4, 2016

Thanks!
We could definately replace it.

@bomsy bomsy deleted the outline-matches branch November 4, 2016 00:16
@jasonLaster
Copy link
Contributor

Awesome. Let's open a new PR to make it consistent. Also less scary

@bomsy
Copy link
Contributor Author

bomsy commented Nov 4, 2016

ok :)

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.

None yet

2 participants