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

[web] [WIP] Mitmweb options #2430

Merged
merged 23 commits into from Jul 20, 2017

Conversation

Projects
None yet
2 participants
@MatthewShao
Contributor

MatthewShao commented Jul 7, 2017

  • Add two columns layout for name-value.
  • Adjust the structure of OptionMaster.
  • Add a basic implementation for PureStringSequenceOption.
  • Complete validation and updateOption logic for all types.
@MatthewShao

Here's the gif for the current UI.
peek 2017-07-15 22-50

Show outdated Hide outdated web/src/js/ducks/options.js
@MatthewShao

This comment has been minimized.

Show comment
Hide comment
@MatthewShao

MatthewShao Jul 18, 2017

Contributor

The new structure is awesome. 🎉 🍰 The error msg below the option names looks good: that's more conspicuous than the tooltip. I did some minor changes:

  1. sort the options by alphabet order, just like the one in our console version.
  2. make the input border turns to red color if its value is wrong.

While I still have a little complain about the StringSequence textarea, now we could not start a new line by pressing Enter. That's awful when we want to input, for example, a list of ignore hosts... Can we fix this?

Contributor

MatthewShao commented Jul 18, 2017

The new structure is awesome. 🎉 🍰 The error msg below the option names looks good: that's more conspicuous than the tooltip. I did some minor changes:

  1. sort the options by alphabet order, just like the one in our console version.
  2. make the input border turns to red color if its value is wrong.

While I still have a little complain about the StringSequence textarea, now we could not start a new line by pressing Enter. That's awful when we want to input, for example, a list of ignore hosts... Can we fix this?

@mhils

This comment has been minimized.

Show comment
Hide comment
@mhils

mhils Jul 20, 2017

Member

ducks/ui/option.js seems to be a duplicate of ducks/ui/optionsEditor.js?

Other than that, LGTM!

Member

mhils commented Jul 20, 2017

ducks/ui/option.js seems to be a duplicate of ducks/ui/optionsEditor.js?

Other than that, LGTM!

@mhils

mhils approved these changes Jul 20, 2017

@mhils mhils merged commit e1d0bc6 into mitmproxy:master Jul 20, 2017

4 checks passed

codecov/patch 92% of diff hit (target 87.91%)
Details
codecov/project 87.95% (+0.03%) compared to 374c27f
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