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

minor project search improvements#3659

Merged
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
jasonLaster:text-search-ux2
Aug 14, 2017
Merged

minor project search improvements#3659
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
jasonLaster:text-search-ux2

Conversation

@jasonLaster
Copy link
Copy Markdown
Contributor

Associated Issue: #1108

Summary of Changes

  • fix storybook
  • tweak colors


.project-text-search .file-result.focused {
background-color: #eeeeee;
background-color: var(--theme-selection-background);
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.

these colors are a small step in the right direction

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 good!

const TextSearch = React.createFactory(_TextSearch);

import _Shortcuts from "./helpers/shortcuts";
const Shortcuts = createFactory(_Shortcuts);
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.

Are we still using this pattern?

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 suppose we should JSX the stories too, but meh

"align-self": "center"
width: "calc(100vw - 100px)",
height: "calc(100vh - 100px)",
margin: "auto",
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.

my gut instinct is to move styles that are not calculated to a class, but it was like this before so...

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.

yeah - it's a silly story, which is where I'm not sure

@codehag
Copy link
Copy Markdown
Contributor

codehag commented Aug 14, 2017

we have a failing test:

*** End BrowserChrome Test Results ***

The following tests failed:

5751 INFO TEST-UNEXPECTED-FAIL | devtools/client/debugger/new/test/mochitest/browser_dbg-search-project.js | undefined assertion name - Got project, expected null

Stack trace:

    chrome://mochikit/content/browser-test.js:test_is:911

    chrome://mochitests/content/browser/devtools/client/debugger/new/test/mochitest/browser_dbg-search-project.js:null:56

    Tester_execTest@chrome://mochikit/content/browser-test.js:735:9

    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7

    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:791:59

Buffered messages finished

SUITE-END | took 164s


./bin/run-mochitests-docker returned exit code 1

@jasonLaster jasonLaster merged commit c1e811b into firefox-devtools:master Aug 14, 2017
@jasonLaster
Copy link
Copy Markdown
Contributor Author

the test is fixed by the other PR

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