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

Make "missing parameter value" message friendlier #2721

Open
arikfr opened this issue Aug 5, 2018 · 18 comments
Open

Make "missing parameter value" message friendlier #2721

arikfr opened this issue Aug 5, 2018 · 18 comments

Comments

@arikfr
Copy link
Member

arikfr commented Aug 5, 2018

Solution

  1. When a required parameter value is missing and the user tries to run a query, the parameter should be marked as missing value (using red border). We might want to accompany this with some animation on the color transition, to guide the user towards the issue?

image

  1. We shouldn't try to execute the query or show the error message we currently show.

Problem

Currently we piggy back the query execution error mechanism to notify the user that they need to enter a value for a parameter when a value is missing.

On a query screen:

image

On a dashboard:

image

The big red message feels very intimidating and I wonder if there is a better way to convey this message?

@kocsmy
Copy link
Collaborator

kocsmy commented Aug 5, 2018

How about changing the design to warning or info so it'll be less intimidating? We can also refine the text to be friendlier: Please set a value for example parameter.

@kocsmy
Copy link
Collaborator

kocsmy commented Aug 6, 2018

We should add the error message next to the input fields.

@arikfr
Copy link
Member Author

arikfr commented Aug 27, 2018

We shouldn't treat this as an error message. It's just a state of the UI. One possible solution is to mark the parameters which miss value with some indication.

@kocsmy
Copy link
Collaborator

kocsmy commented Aug 27, 2018

I meant like an input validation error, Bootstrap supports this by default:
screenshot 2018-08-27 17 59 38

@thoechsmann
Copy link

Would also be good to allow for empty parameters. Sometimes that is intended and I have to make a workaround like enter a text like 'none' when string parameter should be empty.

@kocsmy
Copy link
Collaborator

kocsmy commented Nov 9, 2018

@arikfr I'm wondering if we should move those parameters inputs to Ant. Their error message is similar but placed below the form (which makes more sense to me). Furthermore, the date and time pickers are already in Ant.

image

@arikfr
Copy link
Member Author

arikfr commented Nov 12, 2018

I think that @kravets-levko's implementation of dashboard parameters already switches everything to React, so using Ant won't be a problem.

But --

  1. I'm not sure we should show any message beyond marking the field.
  2. I'm looking for a more holistic definition of the behavior here when trying to run a query (or refresh a dashboard) and some parameter is missing value.

@kocsmy
Copy link
Collaborator

kocsmy commented Nov 12, 2018

  1. I think we should show error message. Traditionally, the UX of error message should 1. show where the error is, 2. tell what the error is, 3. recommend a solution. For example: Missing parameter, please fill. (or something similar)

  2. Can you elaborate on this and what's the problem with the current implementation? The interactions feel quite natural for me and besides moving the error message to its place (next to the field), I can't see serious issues here but I might be missing something.

@arikfr
Copy link
Member Author

arikfr commented Dec 2, 2018

Can you elaborate on this and what's the problem with the current implementation?

It looks ugly and intimidating.

I updated the issue description with the proposed solution. See if you have any comments.

@kocsmy
Copy link
Collaborator

kocsmy commented Dec 7, 2018

Yes, this is better this way. Do you think we should just highlight the input without placing any error message below (like in my proposed design). I think it can be enough just to highlight the field but not 100% convinced.

@arikfr
Copy link
Member Author

arikfr commented Dec 10, 2018

I think for now the highlight is enough. In most cases the issue is missing value, and this is obvious from the empty highlighted field.

@arikfr arikfr added the backlog label Dec 17, 2018
@ranbena
Copy link
Contributor

ranbena commented Jan 15, 2019

I think that best approach is to disable the "Execute" button when not all execute conditions are fulfilled, alongside a clear indication of the disable reason.

(Working on demo)

@arikfr
Copy link
Member Author

arikfr commented Jan 15, 2019

Looking forward :)

@ranbena
Copy link
Contributor

ranbena commented Jan 15, 2019

(ignore the wording and style)
Disallow execution if any param empty.

  1. Toggle off execute button
  2. Show reason tooltip on button
  3. Indicate error on empty parameters. (Error message is optional and might be better without)

screen shot 2019-01-15 at 13 26 41

@arikfr
Copy link
Member Author

arikfr commented Jan 15, 2019

@ranbena makes sense. I also think we can skip the error message, and just use the error indication.

It does mean we will need to duplicate the logic of finding missing values between backend and frontend.

cc: @rauchy

@ranbena
Copy link
Contributor

ranbena commented Jun 18, 2019

When a required parameter value is missing

@arikfr are all params required?

@arikfr
Copy link
Member Author

arikfr commented Jun 18, 2019

@ranbena yes :-) but it will change eventually.

Some additional input:

  1. There might be other issues with a parameter, like wrong value.
  2. Based on the above and to avoid duplicate logic between backend/frontend, how about we change the backend response to have a dictionary of "parameter name" => "error message" that we will use here?

@ranbena
Copy link
Contributor

ranbena commented Jun 18, 2019

  1. There might be other issues with a parameter, like wrong value.
  2. Based on the above and to avoid duplicate logic between backend/frontend, how about we change the backend response to have a dictionary of "parameter name" => "error message" that we will use here?

Sounds good.
Yay my first backend task!

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

Successfully merging a pull request may close this issue.

4 participants