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

Query Builder: disabled inputs should be empty #14888

Open
imincheva opened this issue Oct 7, 2024 · 4 comments · May be fixed by #14973, #14974 or #15011
Open

Query Builder: disabled inputs should be empty #14888

imincheva opened this issue Oct 7, 2024 · 4 comments · May be fixed by #14973, #14974 or #15011
Assignees
Labels
🐛 bug Any issue that describes a bug query-builder ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.

Comments

@imincheva
Copy link

imincheva commented Oct 7, 2024

Issue
image

Steps to reproduce

  1. Start with an empty query.
  2. Create a new group by clicking and/or group button
  3. Select ID column
  4. Select filter, which supports value, e.g. contains
  5. Type in value e.g. 123
  6. Change the selected filter to filter, which doesn't support value, e.g. Empty

Result
The "Value" input text still says 123.

Expected result
The input text from "Value" input should be removed and the input should be empty.

Originally posted by @imincheva in #14647 (comment)

@imincheva imincheva changed the title **Query Builder: disabled inputs should be empty** Query Builder: disabled inputs should be empty Oct 7, 2024
@imincheva imincheva added 🐛 bug Any issue that describes a bug query-builder labels Oct 7, 2024
@georgianastasov georgianastasov added 🛠️ status: in-development Issues and PRs with active development on them and removed 🆕 status: new labels Oct 25, 2024
@georgianastasov georgianastasov added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels Oct 29, 2024
@teodosiah
Copy link
Contributor

@sdimchevski
Currently, the search value input is not cleared not only if we change the filter from binary ('equals') to unary, but also to another binary ('greater than').
Something that concerns me is that if we clear the search value only for the unary operator, the behavior would be inconsistent. For example, changing the operator from binary to binary would not clear the input, but changing it to unary will.
Another thing is that if we set the operator to binary again, the search value would not be restored.
Having these in mind, wouldn't it be better if we either clear the input in all cases (for unary and binary operators) or keep the current behavior?
Also, if we decide to clear it, should we restore the previous value if the operator is changed back from unary to binary?

@sdimchevski
Copy link

@teodosiah from a consistency standpoint, users will quickly understand that changing the operator will always reset the input, making the component more predictable. Since consistency and predictability are our top priorities, clearing the input value in all cases (whether for unary or binary operators) offers the simplest and most effective approach. This also simplifies the code by eliminating the need to manage and store previous values. In the future though, we could consider adding a warning to help mitigate the risk of data loss.

@georgianastasov georgianastasov added 🛠️ status: in-development Issues and PRs with active development on them ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. 🛠️ status: in-development Issues and PRs with active development on them labels Nov 4, 2024
@ivanvpetrov
Copy link
Contributor

ivanvpetrov commented Nov 5, 2024

@sdimchevski
1 In my opinion, from user stand point it's irritating to loose your input data.
2 I don't see any real reason to clear the search value, since it's 'left over' existence won't cause any unexpected behavior.
3 In Excel, which I believe is our reference point, changing the operator doesn't clear the input.

I have a working solution to this issue, which I'll be happy to share. My question beforehand is, from you perspective which is the best presentation of a disabled field? (Or maybe suggest something else):

image

Thank you

P.S.
There is a presentational PR for the approach I'm suggesting #15011

@sdimchevski
Copy link

@ivanvpetrov I'm concerned about complexity or causing confusion, that's why I suggested a simpler, more predictable approach but you may be right - users who frequently switch between unary and binary operators might find it annoying to re-enter their value, especially if they’re experimenting with different operators to build or refine queries.
The closest to an effective disabled state is number 2 though we may consider using “Disabled” or “Not Available,” which explicitly conveys that the field is disabled. This will make it unmistakably clear to the user that the input is currently not in use and cannot be interacted with. N/A is fine too.

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