From dba16ee9337113e59132297710fefec20a386a71 Mon Sep 17 00:00:00 2001 From: Alejandro Ventura De Moya Date: Tue, 8 Sep 2026 14:21:10 -0700 Subject: [PATCH 1/3] FIREFLY-2085 refactor FootprintToolUI as a functional component --- src/firefly/js/drawingLayers/FootprintTool.js | 11 +- .../js/drawingLayers/FootprintToolUI.jsx | 262 ++++++++---------- 2 files changed, 128 insertions(+), 145 deletions(-) diff --git a/src/firefly/js/drawingLayers/FootprintTool.js b/src/firefly/js/drawingLayers/FootprintTool.js index 4c2fff3389..ac8462c4e4 100644 --- a/src/firefly/js/drawingLayers/FootprintTool.js +++ b/src/firefly/js/drawingLayers/FootprintTool.js @@ -395,13 +395,20 @@ function getLayerChanges(drawLayer, action) { return retV; case MODIFY_CUSTOM_FIELD: - const {fpText, fpTextLoc, angleDeg} = action.payload.changes; + const {fpText, fpTextLoc, angleDeg, activePlotId} = action.payload.changes; if (plotIdAry) { if (!isNil(angleDeg)) { return updateFootprintAngle(angleDeg, dd[DataTypes.DATA], plotIdAry); } else { - return updateMarkerText(fpText, fpTextLoc, dd[DataTypes.DATA], plotIdAry); + // updateMarkerText skips plots with no footprint drawobj - only track the title when + // the label was applied to the plot the edit came from, so a no-op update can't reset it + const textApplied = isGoodPlot(activePlotId) && + !isEmpty(get(dd, [DataTypes.DATA, activePlotId])); + + // the layer title tracks the label; an empty label falls back to the title it was created with + return {...updateMarkerText(fpText, fpTextLoc, dd[DataTypes.DATA], plotIdAry), + ...(textApplied && {title: fpText || drawLayer.defaultTitle})}; } } break; diff --git a/src/firefly/js/drawingLayers/FootprintToolUI.jsx b/src/firefly/js/drawingLayers/FootprintToolUI.jsx index 81c4e4a0a5..8e06bd375f 100644 --- a/src/firefly/js/drawingLayers/FootprintToolUI.jsx +++ b/src/firefly/js/drawingLayers/FootprintToolUI.jsx @@ -2,11 +2,12 @@ * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ -import {Chip, Stack, Typography} from '@mui/joy'; -import React, {PureComponent} from 'react'; +import {Chip, Stack, Tooltip, Typography} from '@mui/joy'; +import React, {useState} from 'react'; import PropTypes from 'prop-types'; import {flux} from '../core/ReduxFlux.js'; import {ListBoxInputFieldView} from '../ui/ListBoxInputField.jsx'; +import {useStoreConnector} from '../ui/SimpleComponent.jsx'; import {dispatchModifyCustomField} from '../visualize/DrawLayerDispatch'; import {formatWorldPt} from '../visualize/ui/WorldPtFormat.jsx'; import {DRAWING_LAYER_KEY} from '../visualize/VisConst'; @@ -17,170 +18,145 @@ import {ANGLE_UNIT} from '../visualize/draw/MarkerFootprintObj.js'; import {currentP, getDrawLayerById} from '../visualize/PlotViewUtil.js'; import CsysConverter from '../visualize/CsysConverter.js'; import {InputFieldView} from '../ui/InputFieldView.jsx'; -import {isNil} from 'lodash'; import {sprintf} from '../externalSource/sprintf'; import {FixedPtControl} from './FixedPtControl.jsx'; -export const getFootprintToolUIComponent = (drawLayer,pv) => ; +// key by plotId to handle switching between images +export const getFootprintToolUIComponent = (drawLayer,pv) => + ; export const defaultFootprintTextLoc = TextLocation.REGION_SE; const precision = '%.1f'; -class FootprintToolUI extends PureComponent { - constructor(props) { - super(props); - - const fpObj = this.props.drawLayer?.drawData?.data?.[this.props.pv.plotId] ?? {}; - const {angle = 0.0, angleUnit = ANGLE_UNIT.radian, text = '', textLoc = defaultFootprintTextLoc} = fpObj; - const angleDeg = `${formatAngle(convertAngle(angleUnit.key, 'deg', angle))}`; - const {fpInfo} = this.props.drawLayer; - const {currentPt} = fpObj?.actionInfo ?? {}; - const {plot}= currentP(this.props.pv.plotId); - - this.csys = CsysConverter.make(plot); - this.state = {fpText: text, fpTextLoc: textLoc.key, angleDeg, fpInfo, - currentPt: this.csys.getWorldCoords(currentPt), isValidAngle: true}; - this.changeFootprintText = this.changeFootprintText.bind(this); - this.changeFootprintTextLocation = this.changeFootprintTextLocation.bind(this); - this.changeFootprintAngle = this.changeFootprintAngle.bind(this); - } +export function FootprintToolUI({drawLayer, pv}) { + const {drawLayerId, fpInfo} = drawLayer; + const {plotId} = pv; + + const {hasData, currentPt, angle, angleUnit, angleFromUI, text, textLoc} = useStoreConnector(() => { + const dl = getDrawLayerById(flux.getState()[DRAWING_LAYER_KEY], drawLayerId); + const fpObj = dl?.drawData?.data?.[plotId]; + const {angle = 0.0, angleUnit = ANGLE_UNIT.radian, angleFromUI = false, + text = '', textLoc = defaultFootprintTextLoc} = fpObj ?? {}; + return {hasData: Boolean(fpObj), currentPt: fpObj?.actionInfo?.currentPt, + angle, angleUnit, angleFromUI, text, textLoc}; + }, [drawLayerId, plotId]); + + const csys = CsysConverter.make(currentP(plotId)?.plot); + const derivedCenterPt = currentPt && csys?.getWorldCoords(currentPt); + const derivedCenterKey = derivedCenterPt + ? `${derivedCenterPt.x},${derivedCenterPt.y},${derivedCenterPt.cSys}` : ''; + + const storeAngleDeg = formatAngle(convertAngle(angleUnit.key, 'deg', angle)); + + const storeFpKey = `${storeAngleDeg}|${currentPt?.x},${currentPt?.y}`; + + const [angleDeg, setAngleDeg] = useState(storeAngleDeg); + const [centerPt, setCenterPt] = useState(derivedCenterPt); + const [lastCenterKey, setLastCenterKey] = useState(derivedCenterKey); + const [lastStoreFpKey, setLastStoreFpKey] = useState(storeFpKey); + const [fpText, setFpText] = useState(text); + const [fpTextLoc, setFpTextLoc] = useState(textLoc); + + const isValidAngle = !isNaN(parseFloat(angleDeg)); + + if (hasData) { + if (derivedCenterKey && derivedCenterKey !== lastCenterKey) { + setLastCenterKey(derivedCenterKey); + setCenterPt(derivedCenterPt); + } + // these are local so an in-progress edit survives, but follow the store when it disagrees. + // a local edit dispatches synchronously, so it already matches + if (text !== fpText) setFpText(text); - componentWillUnmount() { - this.iAmMounted= false; - if (this.removeListener) this.removeListener(); - } + if (textLoc !== fpTextLoc) setFpTextLoc(textLoc); - componentDidMount() { - this.iAmMounted= true; - this.removeListener= flux.addListener(() => this.stateUpdate()); - } - - stateUpdate() { - const dl = getDrawLayerById(flux.getState()[DRAWING_LAYER_KEY], this.props.drawLayer.drawLayerId); - - if (dl && this.iAmMounted) { - const crtFpObj = dl?.drawData?.data?.[this.props.pv.plotId]; - - if (crtFpObj) { - let {currentPt} = crtFpObj?.actionInfo ?? {}; - if (currentPt) { - currentPt = this.csys.getWorldCoords(currentPt); - if (currentPt !== this.state.currentPt) { - this.setState({currentPt}); - } - } - - if (!crtFpObj?.angleFromUI) { - var {angle = 0.0, angleUnit = ANGLE_UNIT.radian} = crtFpObj; - - angle = convertAngle(angleUnit.key, 'deg', angle); - this.setState({angleDeg: `${formatAngle(angle)}`, isValidAngle: true}); - } - - var {text = '', textLoc = defaultFootprintTextLoc} = crtFpObj; - if (text !== this.state.fpText) { - this.setState({fpText: text}); - } - if (textLoc.key !== this.state.fpTextLoc) { - this.setState({fpTextLoc: textLoc.key}); - } + // storeFpKey changes when the footprint itself moves (drag or rotate) - on those, an uncommitted + // (invalid) angle entry is discarded, since only a valid entry ever reached the store + if (storeFpKey !== lastStoreFpKey) { + setLastStoreFpKey(storeFpKey); + if (!angleFromUI || !isValidAngle) { + setAngleDeg(storeAngleDeg); } } } - changeFootprintText(ev) { - let fpText = ev?.target?.value; + const changeFootprintText = (ev) => { + const newText = ev?.target?.value ?? ''; - if (isNil(fpText) || !fpText) { - const dl = getDrawLayerById(flux.getState()[DRAWING_LAYER_KEY], this.props.drawLayer.drawLayerId); + setFpText(newText); + dispatchModifyCustomField(drawLayerId, + {fpText: newText, fpTextLoc, activePlotId: plotId}, + plotId); + }; - fpText = ''; - this.props.drawLayer.title = dl?.defaultTitle; - } else { - this.props.drawLayer.title = fpText; - } - this.setState({fpText}); + const changeFootprintTextLocation = (ev, newLocKey) => { + const newLoc = TextLocation.get(newLocKey) ?? defaultFootprintTextLoc; - dispatchModifyCustomField( this.props.drawLayer.drawLayerId, - {fpText, fpTextLoc: TextLocation.get(this.state.fpTextLoc), activePlotId: this.props.pv.plotId }, - this.props.pv.plotId); - } + setFpTextLoc(newLoc); + dispatchModifyCustomField(drawLayerId, + {fpText, fpTextLoc: newLoc, activePlotId: plotId}, + plotId); + }; - changeFootprintTextLocation(ev,fpTextLoc ) { + const changeFootprintAngle = (ev) => { + const newAngleDeg = ev?.target?.value ?? ''; + const valid = !isNaN(parseFloat(newAngleDeg)); - this.setState({fpTextLoc}); - dispatchModifyCustomField( this.props.drawLayer.drawLayerId, - {fpText: this.state.fpText, fpTextLoc: TextLocation.get(fpTextLoc), activePlotId: this.props.pv.plotId }, - this.props.pv.plotId); - } - - changeFootprintAngle(ev) { - let angleDeg = ev?.target?.value; - let isValidAngle = true; + setAngleDeg(newAngleDeg); - if (isNaN(parseFloat(angleDeg))) { - if (!angleDeg) angleDeg = ''; - isValidAngle = false; - } - this.setState({isValidAngle, angleDeg}); - if (isValidAngle) { - dispatchModifyCustomField(this.props.drawLayer.drawLayerId, {angleDeg, activePlotId: this.props.pv.plotId }, - this.props.pv.plotId); + if (valid) { + dispatchModifyCustomField(drawLayerId, {angleDeg: newAngleDeg, activePlotId: plotId}, plotId); } - } + }; + + const textOnLink = fpInfo?.fromFile ? `Add another ${fpInfo.fromFile}` + : fpInfo?.fromRegionAry + ? `Add another ${drawLayer.title}` + : `Add another ${fpInfo?.footprint}${fpInfo?.instrument ? ' '+fpInfo.instrument : ''}`; + + return ( + + + Center: + {formatWorldPt(centerPt,3,false)} + + + - render() { - const {isValidAngle, angleDeg, fpText, fpTextLoc, fpInfo} = this.state; - const textOnLink = fpInfo?.fromFile ? `Add another ${fpInfo.fromFile}` - :fpInfo?.fromRegionAry - ? `Add another ${this.props.drawLayer.title}` - : `Add another ${fpInfo?.footprint} ${fpInfo?.instrument}`; - - - return ( - - - Center: - {formatWorldPt(this.state.currentPt,3,false)} - - - - - - - - - - - addFootprintDrawLayer(this.props.pv, this.state.fpInfo)}> - {textOnLink} - - - + + + + + addFootprintDrawLayer(pv, fpInfo)}> + {textOnLink} + + - ); - } + + ); } From 8a70467ff8852f680e34b680ce86956e588755f4 Mon Sep 17 00:00:00 2001 From: Alejandro Ventura De Moya Date: Tue, 8 Sep 2026 18:47:42 -0700 Subject: [PATCH 2/3] copy edit comments --- src/firefly/js/drawingLayers/FootprintToolUI.jsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/firefly/js/drawingLayers/FootprintToolUI.jsx b/src/firefly/js/drawingLayers/FootprintToolUI.jsx index 8e06bd375f..786c1ab894 100644 --- a/src/firefly/js/drawingLayers/FootprintToolUI.jsx +++ b/src/firefly/js/drawingLayers/FootprintToolUI.jsx @@ -60,19 +60,23 @@ export function FootprintToolUI({drawLayer, pv}) { const isValidAngle = !isNaN(parseFloat(angleDeg)); if (hasData) { + // update the displayed center as the footprint moves. compare positions as strings. + // an empty key means there was no plot or stored center - in that case keep showing + // the last position instead of blanking the readout. if (derivedCenterKey && derivedCenterKey !== lastCenterKey) { setLastCenterKey(derivedCenterKey); setCenterPt(derivedCenterPt); } - // these are local so an in-progress edit survives, but follow the store when it disagrees. - // a local edit dispatches synchronously, so it already matches + // keep local state and take the store's value when it differs. + // the dispatch preserves in-progress edits. if (text !== fpText) setFpText(text); if (textLoc !== fpTextLoc) setFpTextLoc(textLoc); - // storeFpKey changes when the footprint itself moves (drag or rotate) - on those, an uncommitted - // (invalid) angle entry is discarded, since only a valid entry ever reached the store + // refresh the angle box when the stored angle or position changes (drag or rotation), but not + // while angleFromUI is set: that flag stays set after an edit rotates the footprint, so the + // field isn't rewritten between keystrokes. an entry that isn't a number is dropped. if (storeFpKey !== lastStoreFpKey) { setLastStoreFpKey(storeFpKey); if (!angleFromUI || !isValidAngle) { From 1e7801d05eba43f92e010b9d2cf1148f176ad180 Mon Sep 17 00:00:00 2001 From: Alejandro Ventura De Moya Date: Wed, 9 Sep 2026 07:21:00 -0700 Subject: [PATCH 3/3] clarify comments --- src/firefly/js/drawingLayers/FootprintTool.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/firefly/js/drawingLayers/FootprintTool.js b/src/firefly/js/drawingLayers/FootprintTool.js index ac8462c4e4..6890c0235e 100644 --- a/src/firefly/js/drawingLayers/FootprintTool.js +++ b/src/firefly/js/drawingLayers/FootprintTool.js @@ -401,12 +401,11 @@ function getLayerChanges(drawLayer, action) { if (!isNil(angleDeg)) { return updateFootprintAngle(angleDeg, dd[DataTypes.DATA], plotIdAry); } else { - // updateMarkerText skips plots with no footprint drawobj - only track the title when - // the label was applied to the plot the edit came from, so a no-op update can't reset it + // only update the layer title if there is a footprint in the active plot const textApplied = isGoodPlot(activePlotId) && !isEmpty(get(dd, [DataTypes.DATA, activePlotId])); - // the layer title tracks the label; an empty label falls back to the title it was created with + // the layer title tracks the label; an empty label falls back to the title with which it was created return {...updateMarkerText(fpText, fpTextLoc, dd[DataTypes.DATA], plotIdAry), ...(textApplied && {title: fpText || drawLayer.defaultTitle})}; }