Parameter feedback - #5 Unsaved indication #4322
Conversation
ranbena
force-pushed
the
param-feedback-4
branch
from
cd69ffc
to
fbfa980
Compare
ranbena
force-pushed
the
param-feedback-5
branch
from
f3763fa
to
eccbe7d
Compare
ranbena
force-pushed
the
param-feedback-4
branch
from
fbfa980
to
dd588f4
Compare
ranbena
force-pushed
the
param-feedback-5
branch
from
eccbe7d
to
276ca4c
Compare
ranbena
force-pushed
the
param-feedback-4
branch
from
dd588f4
to
557c9ba
Compare
ranbena
force-pushed
the
param-feedback-5
branch
from
11f5e22
to
65dc87a
Compare
ranbena
force-pushed
the
param-feedback-4
branch
from
557c9ba
to
7e3a02a
Compare
ranbena
force-pushed
the
param-feedback-5
branch
from
65dc87a
to
633821d
Compare
|
Just one thing in case you already had thoughts about it: It makes sense to have the same indication in the future for modified parameters (not sure if that will be a quick thing, so no need for it now 😅) |
Definitely. It's really not clear that changing a parameter setting requires saving the query. |
ranbena
force-pushed
the
param-feedback-5
branch
from
633821d
to
1c92bb2
Compare
| const feedback = ( | ||
| <> | ||
| Unsaved{' '} | ||
| <Tooltip title='Click the "Save" button to preserve this parameter.'> |
There was a problem hiding this comment.
@susodapop any better suggestions for this text?
There was a problem hiding this comment.
'Click the "Save" button to make this the default value for this parameter.'
There was a problem hiding this comment.
But it's actually for the parameter itself - not its value.
The case is when a new parameter is typed into the query but ain't saved to DB till the user hits the "save" button.
There was a problem hiding this comment.
In that case it's no different than any other change to a query. Is a special message necessary? I like the yellow outline. Perhaps the effect could be complemented by a similar change in color to the Save button.
There was a problem hiding this comment.
It's not a must, but the asterisk indication imo is not clear enough for changes that are represented below it (including parameter setting changes which is out of scope).
I encourage you to experience it and see for your self if it provides a better user experience.
There was a problem hiding this comment.
I agree the asterisk is sub-optimal. Could we replace it with an indicator that is more obvious?
If you keep the tooltip then the text you've written above is fine.
There was a problem hiding this comment.
One way to go about it is not show or disable the Save button when query isn't dirty.
So when it is, there would be a clear indication that there are changes to be saved.
Another reason I wanted this inline indication was so that it would be clear which parameter corresponds to the one added. So "Unsaved" could be substituted with "New parameter" with a green color instead.
@arikfr how do you feel about this feature?
There was a problem hiding this comment.
I think that this code needs a little bit refactoring (make parseQuery a regular function, move getUnsavedParameters to Query service, and so on - nothing specific to suggest ATM). But I decided that it's good enough for now (we can do refactoring later). So approved 👍
* 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)
* 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)
* Parameter feedback - #2 Client errors in query page * Added cypress test * Fixed percy screenshot * Safer touched change * Parameter feedback - #3 Added in Widgets (#4320) * 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)
Description
Riding on the new param feedback feature, here's an indication for unsaved parameters as they are added.