Skip to content
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

Conditions after free text #22

Open
gausie opened this issue Jun 19, 2018 · 4 comments
Open

Conditions after free text #22

gausie opened this issue Jun 19, 2018 · 4 comments

Comments

@gausie
Copy link

gausie commented Jun 19, 2018

this.string = `${conditionStr} ${this.getAllText()}`.trim();

I know it's a tiny thing, but I think it makes more sense to have conditions follow rather than precede the freetext. I'm happy to submit a PR but would you rather it was an option or a default? Would it be an argument to toString()?

@mericsson
Copy link
Contributor

mericsson commented Jun 21, 2018

Hey @gausie great idea. I hesitate to change the default arrangement. It seems to work well for us at Mixmax and I think that would warrant a major version semver.

I think a PR with toString({ textBeforeConditions }) would work. Please include a test! 😄

@abergs
Copy link

abergs commented Jul 1, 2018

How about having the freetext / options in it's original position?

Image I type this into the search field:

foo -bar prop1:false

and when pressing enter it (confusing to the user) changes the input to:

prop1:false -bar foo

etc.

I now have to discover/find my freetext/props to change it. So maybe adding a third option: toString({ originalOrder})

@mericsson
Copy link
Contributor

mericsson commented Jul 2, 2018

Hey @abergs I think that's a great idea! That's something we noticed here at Mixmax and would love to improve. Assuming it is done well, I think that should be the default functionality. Would be a sweet PR if you're keen to do it!

@gausie
Copy link
Author

gausie commented Jul 5, 2018

I'm going to try to pick this up next week, focusing on original order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants