Skip to content

Firefly-2016: fixed FieldGroupConnector to properly initialize in keepState case #1996

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.

Sign up for GitHub

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

Open
robyww wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Firefly-2016: fixed FieldGroupConnector to properly initialize in keepState case #1996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 24 additions & 10 deletions src/firefly/java/edu/caltech/ipac/firefly/server/query/VosiCapabilityRetrieve.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Stack;

/**
Expand Down Expand Up @@ -78,15 +81,17 @@ static class CapabilityHandle extends DefaultHandler {
private boolean canUseCoord2= false;
private boolean canUseCoordSys= false;
private boolean canUseDistance= false;
private final List<String> outputFormats = new ArrayList<>();
boolean foundTAP= false;
boolean foundFeatures= false;
boolean foundOutputFormat= false;
boolean geoFeaturesProcess = false;

CapabilityHandle() { }

public TapCapability getTapCapability() {
if (geoFeaturesProcess) {
return new TapCapability( canUpload, true,
return new TapCapability(outputFormats, canUpload, true,
canUsePoint, canUseCircle, canUseBox,
canUsePolygon, canUseRegion, canUseContains,
canUseIntersects, canUseArea, canUseCentroid,
Expand All @@ -110,6 +115,9 @@ public void startElement(String uri, String localName, String qName, Attributes
if (qName.equals("uploadMethod") && confirmAttribute(att,"ivo-id", uploadHttp)) {
canUpload= true;
}
if (qName.equals("outputFormat")) {
foundOutputFormat= true;
}
}
currValue.setLength(0);
currElement.push(qName);
Expand All @@ -119,8 +127,7 @@ public void startElement(String uri, String localName, String qName, Attributes
public void endElement(String uri, String localName, String qName) {
if (foundTAP && qName.equals("capability")) foundTAP= false;
if (foundFeatures && qName.equals("languageFeatures")) foundFeatures= false;


if (foundOutputFormat && qName.equals("outputFormat")) foundOutputFormat= false;


String value= currValue.toString();
Expand All @@ -142,6 +149,9 @@ public void endElement(String uri, String localName, String qName) {
}
geoFeaturesProcess = true;
}
if (foundTAP && foundOutputFormat && qName.equals("mime")) {
outputFormats.add(value);
}
if (!currElement.empty()) currElement.pop();
}

Expand All @@ -165,15 +175,19 @@ private static boolean confirmAttribute(Attributes att, String attName, String[]

public record Capabilities( TapCapability tapCapability) {}

public record TapCapability( boolean canUpload, boolean foundGeoLanguageFeatures,
boolean canUsePoint, boolean canUseCircle, boolean canUseBox,
boolean canUsePolygon, boolean canUseRegion, boolean canUseContains,
boolean canUseIntersects, boolean canUseArea, boolean canUseCentroid,
boolean canUseCoord1, boolean canUseCoord2, boolean canUseCoordSys,
boolean canUseDistance) {}
public record TapCapability(
List<String> outputFormats,
boolean canUpload, boolean foundGeoLanguageFeatures,
boolean canUsePoint, boolean canUseCircle, boolean canUseBox,
boolean canUsePolygon, boolean canUseRegion, boolean canUseContains,
boolean canUseIntersects, boolean canUseArea, boolean canUseCentroid,
boolean canUseCoord1, boolean canUseCoord2, boolean canUseCoordSys,
boolean canUseDistance) {}

private static TapCapability makeDefaultTapCapability(boolean canUpload) {
return new TapCapability(canUpload, false,
return new TapCapability(
Collections.emptyList(),
canUpload, false,
true,true,false,
true,false,true,
false,false,false,
Expand Down
4 changes: 2 additions & 2 deletions src/firefly/js/fieldGroup/FieldGroupCntlr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ export function dispatchMountComponent(groupKey,fieldKey,mounted,value,initField
* @param {Object} payload
* @param {String} payload.fieldKey the field Key
* @param {String} payload.groupKey group key
* @param {String} payload.value value can be anything including a promise or function
* @param {boolean} payload.valid - true if valid, default to true
* @param {String} [payload.value] value can be anything including a promise or function
* @param {boolean} [payload.valid] - true if valid, default to true
*/
export function dispatchValueChange(payload) {
flux.process({type: VALUE_CHANGE, payload});
Expand Down
37 changes: 19 additions & 18 deletions src/firefly/js/ui/FieldGroupConnector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const STORE_OMIT_LIST= ['fieldKey', 'groupKey', 'initialState', 'fireReducer',
'confirmValue', 'confirmValueOnInit', 'forceReinit', 'mounted'];

function buildViewProps(fieldState,props,fieldKey,groupKey,value='') {
const {message= '', valid= true, visible= true, value:ignoreValue, displayValue= '',
const {message= '', valid= true, visible= true, displayValue= '',
tooltip= '', validator= defValidatorFunc, ...rest}= fieldState;
const propsClean= Object.keys(props).reduce( (obj,k)=> {
if (isDefined(props[k]) && k!=='value') obj[k]=props[k];
Expand Down Expand Up @@ -57,12 +57,22 @@ function hasNewKeys(infoRef,fieldKey,groupKey) {
return isInit(infoRef,fieldKey,groupKey) && prevFieldKey && prevGroupKey;
}

/**
* @param {String} groupKey
* @param {Object} initialState
* @param {Object} props
* @param {ConfirmValueFunc} confirmValueOnInit
* @return {Object}
*/
function doGetInitialState(groupKey, initialState, props, confirmValueOnInit= defaultConfirmValue) {
const {fieldKey}= props;
const {keepState=false}= getFieldGroupState(groupKey) ?? {};
const storeField= get(FieldGroupUtils.getGroupFields(groupKey), [fieldKey]);
const initS= !keepState ? (initialState || storeField || {}) : (storeField || initialState || {});
return {...initS, value: confirmValueOnInit(initS.value,props,initialState,initS)};
const storeField= FieldGroupUtils.getGroupFields(groupKey)[fieldKey] ?? {};
const computedState= keepState
? {...initialState, ...storeField}
: (initialState ?? storeField);
const value= confirmValueOnInit(computedState.value,props,initialState,computedState);
return {...computedState, value};
}


Expand Down Expand Up @@ -102,13 +112,6 @@ export const fgConnectPropsTypes= {
confirmValue: PropTypes.func
};

/**
* Minimal set of properties ot use for useFieldGroupConnector
*/
export const fgMinPropTypes= {
fieldKey: PropTypes.string.isRequired,
groupKey: PropTypes.string, // normally passed in context
};

/**
* @name ConfirmValueFunc
Expand All @@ -118,6 +121,7 @@ export const fgMinPropTypes= {
* @param {*} value
* @param {Object} props
* @param {Object} state
* @param {Object|undefined} computedState
* @returns *
*/

Expand Down Expand Up @@ -160,16 +164,13 @@ export const useFieldGroupConnector= (props) => {
}
}

const getInitialState= () => doGetInitialState(groupKey, initialState, props, (confirmValueOnInit||confirmValue));
const getInitialState= () => doGetInitialState(groupKey, initialState, props, (confirmValueOnInit ?? confirmValue));

const [fieldState, setFieldState] = useState(() => getInitialState());

const fireValueChange= (payload) => dispatchValueChange({...payload, fieldKey,groupKey});
const value= confirmValue ? confirmValue(fieldState.value,props,fieldState) : fieldState.value;

const effectChangeAry= [fieldKey, groupKey, fieldState];
if (confirmValue) effectChangeAry.push(value); // only need to watch value in this case


useEffect(() => {
if (doingInit) { // called the first time or when fieldKey or groupKey change
Expand Down Expand Up @@ -205,11 +206,11 @@ export const useFieldGroupConnector= (props) => {
if (!gState || !gState.mounted || !get(gState,['fields',fieldKey])) return;
if (fieldState !== gState.fields[fieldKey]) setFieldState(gState.fields[fieldKey]);
});
}, effectChangeAry);
}, [fieldKey, groupKey, fieldState,confirmValue]);

useEffect(() => { // only run on dismount
return () => dispatchMountComponent( groupKey, fieldKey, false);
}, []);
}, []); // eslint-disable-line react-hooks/exhaustive-deps

useEffect(() => {
if (doingInit || isUndefined(forceValid)) return;
Expand All @@ -221,7 +222,7 @@ export const useFieldGroupConnector= (props) => {
f.validator= f.validator ?? props.validator;
if (canValidate(f)) fireValueChange({...callValidator(f)});
}
}, [forceValid]);
}, [forceValid]); // eslint-disable-line react-hooks/exhaustive-deps

return {
fireValueChange, viewProps: buildViewProps(fieldState,props,fieldKey,groupKey, value), fieldKey, groupKey
Expand Down
5 changes: 1 addition & 4 deletions src/firefly/js/ui/QuantityInputField.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,7 @@ export const QuantityInputField = memo((props) => {
});

useEffect(() => {
if (!viewProps?.unit && initUnit) {
// if unit is missing from store, set it from initialState
fireValueChange({unit: initUnit});
} else if ((viewProps?.value || viewProps?.value===0) && viewProps?.unit && !viewProps?.displayValue) {
if ((viewProps?.value || viewProps?.value===0) && viewProps?.unit && !viewProps?.displayValue) {
// if value changed in the store (for e.g. by a setFieldValue()) and there's no displayValue, set it
const newDisplayValue = convertQuantityUnits(viewProps.value, quantityBaseUnit, viewProps.unit);
fireValueChange({displayValue: newDisplayValue});
Expand Down