-
Notifications
You must be signed in to change notification settings - Fork 0
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
Unit Edit #12
base: ReadingRangeRejection
Are you sure you want to change the base?
Unit Edit #12
Conversation
It would suggest that the minimum and maximum value input boxes be on the same row rather than a different row.
The improvements suggested by Steve to @mmehta2669 work, apply to this as well, like the default maximum and minimum should not be hardcoded, instead be the maximum and minimum value in javascript Also, @mmehta2669 had similar changes on the branch unitMaxMinClient, I would request you to coordinate with Mark on this |
Changes made as requested from comment @Rakesh-Ranga-Buram. The changes are in the second commit. |
@@ -46,8 +46,10 @@ export interface UnitEditData { | |||
displayable: DisplayableType; | |||
preferredDisplay: boolean; | |||
note: string; | |||
defaultMeterMinimumValue:-999999999999, | |||
defaultMeterMaximumValue:999999999999, | |||
defaultMeterMinimumValue: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole section UnitEditData can be commented out as it is not used elsewhere in the application
{/*Disable checks box */} | ||
<FormGroup> | ||
<Label for='disableChecks'>{translate('disable.checks')}</Label> | ||
<Input | ||
id='disableChecks' | ||
name='disableChecks' | ||
type='select' | ||
value={state.note} | ||
onChange={e => handleStringChange(e)}> | ||
<option value=''> Select an option </option> | ||
<option value='No, only reject the bad reading(s)'> No, only reject the bad reading(s) </option> | ||
<option value='No, reject all readings in batch'> No, reject all readings in batch </option> | ||
<option value='Yes, do not check values'> Yes, do not check value </option> | ||
</Input> | ||
</FormGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Disable checks, the expectation is to create an enum and options should loop through the enum and translate the enum keys to display values of the options
"default.meter.minimum.value": "Default meter minimum reading value check", | ||
"default.meter.maximum.value": "Default meter maximum reading value check", | ||
"default.meter.minimum.date": "Default meter minimum reading date check", | ||
"default.meter.maximum.date": "Default meter maximum reading date check", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need to edit date, just edit the maximum and minimum value like
"default.minimum.value": "Default minimum reading value check",
"default.maximum.value": "Default maximum reading value check"
and do the same for translations in different languages with a bulb icon.
Also when you do these changes, the previous instances of these keys must also be changed
And the lines where these translations are written should not change so please revert back the changes done
The date checks are not required, so limit the changes to maximum value, minimum value and disable checks |
Description
Added Default Min/Max values for edit units.
Partially Addresses OpenEnergyDashboard#1307
Type of change
(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
Limitations
Still need to add the 'default disable check' and the 'default date check.