New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP][web] Add tests for UI components #2327

Merged
merged 11 commits into from May 15, 2017

Conversation

Projects
None yet
3 participants
@MatthewShao
Contributor

MatthewShao commented May 11, 2017

  • comon
  • helpers
@mhils

This comment has been minimized.

Show comment
Hide comment
@mhils

mhils May 11, 2017

Member

Looks good! I think the most interesting part will be in components that take flows as props. It probably makes sense to have a central test flow object that gets reused across a set of components for that.

Member

mhils commented May 11, 2017

Looks good! I think the most interesting part will be in components that take flows as props. It probably makes sense to have a central test flow object that gets reused across a set of components for that.

@@ -21,7 +21,7 @@ export default function FileChooser({ icon, text, className, title, onOpenFile }
ref={ref => fileInput = ref}
className="hidden"
type="file"
onChange={e => { e.preventDefault(); if(e.target.files.length > 0) onOpenFile(e.target.files[0]); fileInput = "";}}

This comment has been minimized.

@cle1000

cle1000 May 13, 2017

Contributor

fileInput = "" should be changed to fileInput.value = ""
this call resets the fileInput. without this extra call it is not possible to open the same file twice...

@cle1000

cle1000 May 13, 2017

Contributor

fileInput = "" should be changed to fileInput.value = ""
this call resets the fileInput. without this extra call it is not possible to open the same file twice...

This comment has been minimized.

@MatthewShao

MatthewShao May 13, 2017

Contributor

Oh, that's right. This statement confuses me a lot... Thanks! @cle1000

@MatthewShao

MatthewShao May 13, 2017

Contributor

Oh, that's right. This statement confuses me a lot... Thanks! @cle1000

@mhils

mhils approved these changes May 15, 2017

@mhils mhils merged commit 69c5a0b into mitmproxy:master May 15, 2017

4 checks passed

codecov/patch 100% of diff hit (target 82.55%)
Details
codecov/project 83.52% (+0.97%) compared to cafa094
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment