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

Is valid gps input new & setLayout #1402

Open
wants to merge 21 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
598e3e6
Moving setLayout functions to utils
zhenga5 Nov 19, 2024
372f0d5
Changed Radar Chart Components to use setHelpLayout
zhenga5 Nov 19, 2024
a95dbd5
Formatting +keeping setThreeDLayout as a unique function of ThreeDCom…
zhenga5 Nov 19, 2024
fdd24af
Fixing changes made during testing
zhenga5 Nov 22, 2024
50356f2
Created isValidGPSInputNew that returns boolean and a message of the …
zhenga5 Nov 22, 2024
2ece75f
adapting and testing CreateGroupModalComponent to use the new isValid…
zhenga5 Nov 22, 2024
efc219a
Formatting +keeping setThreeDLayout as a unique function of ThreeDCom…
zhenga5 Nov 30, 2024
2eb4665
Creating new setLayout file in utils for import
zhenga5 Nov 30, 2024
7b5e7da
Adapted CreateGroupModalComponent to use IsValidGPSInputNew
zhenga5 Nov 30, 2024
2ebf879
Adapted EditGroupModalComponent to use isValidGPSInputNew
zhenga5 Nov 30, 2024
d2973a4
Implemented isValidGPSInputNew as a temporary solution and will adapt…
zhenga5 Nov 30, 2024
0ef59f6
Adapted CreateMeterModalComponent to use isValidGPSInputNew
zhenga5 Nov 30, 2024
9784579
Changed return message from isValidGPSInputNew for Component files to…
zhenga5 Dec 2, 2024
deba460
Merge remote-tracking branch 'upstream/development' into isValidGPSIn…
zhenga5 Dec 2, 2024
03a07fd
Merge remote-tracking branch 'origin/isValidGPSInputNew' into isValid…
zhenga5 Dec 2, 2024
a3e8c09
Fixing isValidGpsInputNew Logic to fix error notification for invalid…
zhenga5 Dec 3, 2024
eb66706
Changing to use isValidGPSInputNew
zhenga5 Dec 3, 2024
5d994ab
Changing all adapted files back to isValidGPSInput and deleting tempo…
zhenga5 Dec 3, 2024
db24b73
Formatting + adding translations for error messages
zhenga5 Dec 3, 2024
b12513d
Remove TODO Comments
zhenga5 Dec 5, 2024
87b2885
formatting, much outside this PR
huss Dec 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 7 additions & 59 deletions src/client/app/components/RadarChartComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import { values } from 'lodash';
import * as moment from 'moment';
import { Layout } from 'plotly.js';
import * as React from 'react';
import Plot from 'react-plotly.js';
import { Icons } from 'plotly.js';
Expand All @@ -26,6 +25,7 @@ import getGraphColor from '../utils/getGraphColor';
import { lineUnitLabel } from '../utils/graphics';
import { useTranslate } from '../redux/componentHooks';
import SpinnerComponent from './SpinnerComponent';
import {setHelpLayout} from '../utils/setLayout';

/**
* @returns radar plotly component
Expand Down Expand Up @@ -189,31 +189,13 @@ export default function RadarChartComponent() {
}
}

let layout: Partial<Layout>;
let layout = {};
// TODO See 3D code for functions that can be used for layout and notices.
if (datasets.length === 0) {
// There are no meters so tell user.
// Customize the layout of the plot
// See https://community.plotly.com/t/replacing-an-empty-graph-with-a-message/31497 for showing text not plot.
layout = {
'xaxis': {
'visible': false
},
'yaxis': {
'visible': false
},
'annotations': [
{
'text': `${translate('select.meter.group')}`,
'xref': 'paper',
'yref': 'paper',
'showarrow': false,
'font': {
'size': 28
}
}
]
};
layout = setHelpLayout(translate('select.meter.group'), 28);
} else {
// Plotly scatterpolar plots have the unfortunate attribute that if a smaller number of plotting
// points is done first then that impacts the labeling of the polar coordinate where you can get
Expand All @@ -230,25 +212,7 @@ export default function RadarChartComponent() {
// There is no data so tell user - likely due to date range outside where readings.
// Remove plotting data even though none there is an empty r & theta that gives empty graphic.
datasets.splice(0, datasets.length);
layout = {
'xaxis': {
'visible': false
},
'yaxis': {
'visible': false
},
'annotations': [
{
'text': `${translate('radar.no.data')}`,
'xref': 'paper',
'yref': 'paper',
'showarrow': false,
'font': {
'size': 28
}
}
]
};
layout = setHelpLayout(translate('radar.no.data'),28);
} else {
// Check if all the values for the dates are compatible. Plotly does not like having different dates in different
// scatterpolar lines. Lots of attempts to get this to work failed so not going to allow since not that common.
Expand All @@ -265,25 +229,9 @@ export default function RadarChartComponent() {
// Remove plotting data.
datasets.splice(0, datasets.length);
// The lines are not compatible so tell user.
layout = {
'xaxis': {
'visible': false
},
'yaxis': {
'visible': false
},
'annotations': [
{
'text': `${translate('radar.lines.incompatible')}`,
'xref': 'paper',
'yref': 'paper',
'showarrow': false,
'font': {
'size': 28
}
}
]
};
//Change layout to use function from utils
//Then look at other chart components and do it similar way
layout = setHelpLayout(translate('radar.lines.incompatible'),28);
} else {
// Data available and okay so plot.
// Maximum number of ticks, represents 12 months. Too many is cluttered so this seems good value.
Expand Down
32 changes: 4 additions & 28 deletions src/client/app/components/ThreeDComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import Plot from 'react-plotly.js';
import { Icons } from 'plotly.js';
import { selectSelectedLanguage } from '../redux/slices/appStateSlice';
import Locales from '../types/locales';
import {setHelpLayout} from '../utils/setLayout';

/**
* Component used to render 3D graphics
Expand Down Expand Up @@ -191,7 +192,6 @@ function formatThreeDData(
const readingValue = readings === null ? null : readings.toPrecision(6);
return `${translate('threeD.date')}: ${date}<br>${translate('threeD.time')}: ${time}<br>${unitLabel}: ${readingValue}`;
}));

const formattedData = [{
type: 'surface',
showlegend: false,
Expand All @@ -206,39 +206,13 @@ function formatThreeDData(
return [formattedData, layout];
}

/**
* Utility to get/ set help text plotlyLayout
* @param helpText 3D data to be formatted
* @param fontSize current application state
* @returns plotly layout object.
*/
function setHelpLayout(helpText: string = 'Help Text Goes Here', fontSize: number = 28) {
return {
'xaxis': {
'visible': false
},
'yaxis': {
'visible': false
},
'annotations': [
{
'text': helpText,
'xref': 'paper',
'yref': 'paper',
'showarrow': false,
'font': { 'size': fontSize }
}
]
};
}

/**
* Utility to get / set 3D graphic plotlyLayout
* @param zLabelText 3D data to be formatted
* @param yDataToRender Data range for yaxis
* @returns plotly layout object.
*/
function setThreeDLayout(zLabelText: string = 'Resource Usage', yDataToRender: string[]) {
export function setThreeDLayout(zLabelText: string = 'Resource Usage', yDataToRender: string[]) {
// Convert date strings to JavaScript Date objects and then get dataRange
const dateObjects = yDataToRender.map(dateStr => new Date(dateStr));
const dataMin = Math.min(...dateObjects.map(date => date.getTime()));
Expand Down Expand Up @@ -296,3 +270,5 @@ function setThreeDLayout(zLabelText: string = 'Resource Usage', yDataToRender: s
} as Partial<Plotly.Layout>;
}



Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ export default function CreateGroupModalComponent() {
// If the user input a value then gpsInput should be a string.
// null came from the DB and it is okay to just leave it - Not a string.
if (typeof gpsInput === 'string') {
if (isValidGPSInput(gpsInput)) {
const {validGps,message} = isValidGPSInput(gpsInput);
if (validGps) {
// Clearly gpsInput is a string but TS complains about the split so cast.
const gpsValues = (gpsInput as string).split(',').map((value: string) => parseFloat(value));
// It is valid and needs to be in this format for routing.
Expand All @@ -207,10 +208,7 @@ export default function CreateGroupModalComponent() {
};
// gpsInput must be of type string but TS does not think so so cast.
} else if ((gpsInput as string).length !== 0) {
// GPS not okay. Only true if some input.
// TODO isValidGPSInput currently pops up an alert so not doing it here, may change
// so leaving code commented out.
// showErrorNotification(translate('input.gps.range') + state.gps + '.');
showErrorNotification(message);
inputOk = false;
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/client/app/components/groups/EditGroupModalComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ export default function EditGroupModalComponent(props: EditGroupModalComponentPr
// If the user input a value then gpsInput should be a string
// null came from DB and it is okay to just leave it - Not a String.
if (typeof gpsInput === 'string') {
if (isValidGPSInput(gpsInput)) {
const {validGps,message} = isValidGPSInput(gpsInput);
if (validGps) {
// Clearly gpsInput is a string but TS complains about the split so cast.
const gpsValues = (gpsInput as string).split(',').map((value: string) => parseFloat(value));
// It is valid and needs to be in this format for routing
Expand All @@ -297,10 +298,7 @@ export default function EditGroupModalComponent(props: EditGroupModalComponentPr
latitude: gpsValues[latitudeIndex]
};
} else if ((gpsInput as string).length !== 0) {
// GPS not okay and there since non-zero length value.
// TODO isValidGPSInput currently pops up an alert so not doing it here, may change
// so leaving code commented out.
// showErrorNotification(translate('input.gps.range') + groupState.gps + '.');
showErrorNotification(message);
inputOk = false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ export default class MapCalibrationInfoDisplayComponent extends React.Component<
const longitudeIndex = 1;
if (this.props.currentCartesianDisplay === 'x: undefined, y: undefined') { return; }
const input = this.state.value;
if (isValidGPSInput(input)) {
const {validGps} = isValidGPSInput(input);
if (validGps) {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't your doing but would it work if you had an else clause that showed the error if the GPS is invalid? Up to now the code simply does nothing.

const array = input.split(',').map((value: string) => parseFloat(value));
const gps: GPSPoint = {
longitude: array[longitudeIndex],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ export default function CreateMeterModalComponent(props: CreateMeterModalProps):
// If the user input a value then gpsInput should be a string.
// null came from the DB and it is okay to just leave it - Not a string.
if (typeof gpsInput === 'string') {
if (isValidGPSInput(gpsInput)) {
const {validGps, message} = isValidGPSInput(gpsInput);
if (validGps) {
const gpsValues = gpsInput.split(',').map(value => parseFloat(value));
// It is valid and needs to be in this format for routing.
gps = {
Expand All @@ -133,9 +134,7 @@ export default function CreateMeterModalComponent(props: CreateMeterModalProps):
};
} else if (gpsInput.length !== 0) {
// GPS not okay. Only true if some input.
// TODO isValidGPSInput currently pops up an alert so not doing it here, may change
// so leaving code commented out.
// showErrorNotification(translate('input.gps.range') + state.gps + '.');
showErrorNotification(message);
inputOk = false;
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/client/app/components/meters/EditMeterModalComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ export default function EditMeterModalComponent(props: EditMeterModalComponentPr
// If the user input a value then gpsInput should be a string.
// null came from the DB and it is okay to just leave it - Not a string.
if (typeof gpsInput === 'string') {
if (isValidGPSInput(gpsInput)) {
const {validGps, message} = isValidGPSInput(gpsInput);
if (validGps) {
// Clearly gpsInput is a string but TS complains about the split so cast.
const gpsValues = (gpsInput as string).split(',').map((value: string) => parseFloat(value));
// It is valid and needs to be in this format for routing.
Expand All @@ -116,9 +117,7 @@ export default function EditMeterModalComponent(props: EditMeterModalComponentPr
// gpsInput must be of type string but TS does not think so so cast.
} else if ((gpsInput as string).length !== 0) {
// GPS not okay.
// TODO isValidGPSInput currently tops up an alert so not doing it here, may change
// so leaving code commented out.
// showErrorNotification(translate('input.gps.range') + state.gps + '.');
showErrorNotification(message);
inputOk = false;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/client/app/translations/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ const LocaleTranslationData = {
"failed.to.submit.changes": "Failed to submit changes",
"false": "False",
"gps": "GPS: latitude, longitude",
"gps.missing.comma": "GPS Input is missing a comma",
"gps.many.comma": "GPS Input has too many commas",
"graph": "Graph",
"graph.settings": "Graph Settings",
"graph.type": "Graph Type",
Expand Down Expand Up @@ -698,6 +700,8 @@ const LocaleTranslationData = {
"failed.to.submit.changes": "Échec de l'envoi des modifications",
"false": "Faux",
"gps": "GPS: latitude, longitude\u{26A1}",
"gps.missing.comma": "GPS Input is missing a comma\u{26A1}",
"gps.many.comma": "GPS Input has too many commas\u{26A1}",
"graph": "Graphique",
"graph.settings": "Graph Settings\u{26A1}",
"graph.type": "Type du Diagramme",
Expand Down Expand Up @@ -1221,6 +1225,8 @@ const LocaleTranslationData = {
"failed.to.submit.changes": "No se pudo entregar los cambios",
"false": "Falso",
"gps": "GPS: latitud, longitud",
"gps.missing.comma": "GPS Input is missing a comma\u{26A1}",
"gps.many.comma": "GPS Input has too many commas\u{26A1}",
"graph": "Gráfico",
"graph.settings": "Graph Settings\u{26A1}",
"graph.type": "Tipo de gráfico",
Expand Down
43 changes: 26 additions & 17 deletions src/client/app/utils/calibration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { showErrorNotification } from './notifications';
import { logToServer } from '../redux/actions/logs';
import { DataType } from '../types/Datasources';
import { MapMetadata } from '../types/redux/map';
Expand Down Expand Up @@ -68,7 +67,8 @@ export interface Dimensions {
}

/**
* Returns true if item (meter or group) and map and reasonably defined and false otherwise.
* Returns true if item (meter or group) and map and reasonably defined
* Returns false and a message about the reason the input is invalid otherwise
* @param itemID ID to be used for logging errors
* @param type DataType to distinguish between meter and group
* @param map map info to check
Expand All @@ -78,7 +78,8 @@ export interface Dimensions {
export function itemMapInfoOk(itemID: number, type: DataType, map: MapMetadata, gps?: GPSPoint): boolean {
if (map === undefined) { return false; }
if ((gps === null || gps === undefined) || map.origin === undefined || map.opposite === undefined) { return false; }
if (!isValidGPSInput(`${gps.latitude},${gps.longitude}`)) {
const {validGps} = isValidGPSInput(`${gps.latitude},${gps.longitude}`);
if (!validGps) {
logToServer('error', `Found invalid ${type === DataType.Meter ? 'meter' : 'group'} gps stored in database, id = ${itemID}`)();
return false;
}
Expand Down Expand Up @@ -106,25 +107,33 @@ export function itemDisplayableOnMap(size: Dimensions, point: CartesianPoint): b
* @param input The string to check for GPS values
* @returns true if string is GPS and false otherwise.
*/
export function isValidGPSInput(input: string): boolean {
export function isValidGPSInput(input: string){
let message = '';
let validGps = true;
if (input.indexOf(',') === -1) { // if there is no comma
// TODO It would be nice to tell user that comma is missing but need to check all uses to be sure don't get ''.
zhenga5 marked this conversation as resolved.
Show resolved Hide resolved
return false;
message = translate('gps.missing.comma');
validGps = false;
} else if (input.indexOf(',') !== input.lastIndexOf(',')) { // if there are multiple commas
return false;
message = translate('gps.many.comma');
validGps = false;
}
// Works if value is not a number since parseFloat returns a NaN so treated as invalid later.
const array = input.split(',').map((value: string) => parseFloat(value));
const latitudeIndex = 0;
const longitudeIndex = 1;
const latitudeConstraint = array[latitudeIndex] >= -90 && array[latitudeIndex] <= 90;
const longitudeConstraint = array[longitudeIndex] >= -180 && array[longitudeIndex] <= 180;
const result = latitudeConstraint && longitudeConstraint;
if (!result) {
// TODO It would be nice to return the error and then notify as desired.
showErrorNotification(translate('input.gps.range') + input);
if(validGps){
// Works if value is not a number since parseFloat returns a NaN so treated as invalid later.
const array = input.split(',').map((value: string) => parseFloat(value));
const latitudeIndex = 0;
const longitudeIndex = 1;
const latitudeConstraint = array[latitudeIndex] >= -90 && array[latitudeIndex] <= 90;
const longitudeConstraint = array[longitudeIndex] >= -180 && array[longitudeIndex] <= 180;
const result = latitudeConstraint && longitudeConstraint;
if (!result) {
// TODO It would be nice to return the error and then notify as desired.
zhenga5 marked this conversation as resolved.
Show resolved Hide resolved
//showErrorNotification(translate('input.gps.range') + input);
validGps = false;
message = translate('input.gps.range') + input;
}
}
return result;
return {validGps,message};
}

/**
Expand Down
31 changes: 31 additions & 0 deletions src/client/app/utils/setLayout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */


/**
* Utility to get/ set help text plotlyLayout
* @param helpText 3D data to be formatted
* @param fontSize current application state
* @returns plotly layout object.
*/
export function setHelpLayout(helpText: string = 'Help Text Goes Here', fontSize: number = 28) {
return {
'xaxis': {
'visible': false
},
'yaxis': {
'visible': false
},
'annotations': [
{
'text': helpText,
'xref': 'paper',
'yref': 'paper',
'showarrow': false,
'font': { 'size': fontSize }
}
]
};
}

Loading