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

Parameter feedback - #2 Client errors in query page #4319

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

ranbena
Copy link
Contributor

@ranbena ranbena commented Oct 30, 2019

  • Feature

Description

Now that #4312 brings query result errorData, we can show them inline in context.
This PR is for the query page only
Screen Shot 2019-10-30 at 19 44 58

Notice, since the client is oblivious to the error's meaning (it's evaluated in the server), it can't be responsible for it's hiding. For instance, if a parameter was left empty, it displays "Required parameter". Once the user fills the field in, the client doesn't know that the error has been fulfilled - so it can't hide the error.
This causes a jarring user experience. As a user you expect the error indication to disappear immediately when fixed. To mitigate this I implemented a "touched" flag on each parameter which removes the red indication but keeps the error text visible, until re-evaluated by the server. This works well.
Any change to parameter - be it value or settings - toggles on the "touched" flag. The flag resets when the error data from the server has changed.

To accommodate the error messages, more bottom spacing has been given to the parameters.

@ranbena ranbena self-assigned this Oct 30, 2019
@ranbena ranbena marked this pull request as ready for review October 30, 2019 18:44
Copy link
Collaborator

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please see comments

client/app/components/Parameters.jsx Outdated Show resolved Hide resolved
client/app/components/Parameters.jsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

* Parameter feedback - #3 Added in Widgets

* Added cypress tests

* Making sure widget-level param is selected

* Parameter feedback - #4 Added in Dashboard params (#4321)

* Parameter feedback - #4 Added in Dashboard params

* Added cypress test

* Moved to service

* Parameter feedback - #5 Unsaved indication (#4322)

* Parameter feedback - #5 Unsaved indication

* Added ANGULAR_REMOVE_ME

* Added cypress test

* Fixed percy screenshot

* Some code improvements

* Parameter input feedback - #6 Better value normalization (#4327)
@guidopetri guidopetri merged commit 13e5500 into param-feedback-1 Jul 21, 2023
1 check passed
@guidopetri guidopetri deleted the param-feedback-2 branch July 21, 2023 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants