Merge branch 'main' into 1103-react-query-retrofit

.github/workflows/npm-publish-sites.yml

@@ -3,9 +3,9 @@
 
 name: NPM Publish Sites
 on:
-  release:
-    types:
-      - published
+  push:
+    tags:
+      - v*
 
 jobs:
   # Gather the names of the sites directories and store it as an output variable
@@ -51,15 +51,11 @@ jobs:
           elif [[ "$git_tag" =~ ^[0-9]+\.[0-9]+\.[0-9]+-([a-zA-Z0-9]+)(\.[0-9]+)*$ ]]; then
             npm_tag="${BASH_REMATCH[1]}"
             echo "npm_tag=$npm_tag" >> $GITHUB_ENV
-          # if prerelease is checked, use "prerelease" for the npm tag
-          elif ${{ github.event.release.prerelease }}; then
-            echo "npm_tag=prerelease" >> $GITHUB_ENV
           # or fall back to 'latest' just in case
           else
-            echo "npm_tag=latest" >> $GITHUB_ENV
+            echo "npm_tag=alpha" >> $GITHUB_ENV
           fi
       - uses: JS-DevTools/npm-publish@v2
-        if: ${{ startsWith(github.event.release.tag_name, 'v') }}
         with:
           token: ${{ secrets.NPM_TOKEN }}
           access: public

packages/libs/components/src/map/SemanticMarkers.tsx

@@ -64,7 +64,7 @@ export default function SemanticMarkers({
     // 2023-11: it does seem to be needed for zoom-in animation to work.
     const debouncedUpdateMarkerPositions = debounce(
       updateMarkerPositions,
-      1000
+      animation ? animation.duration : 0
     );
     // call it at least once at the beginning of the life cycle
     debouncedUpdateMarkerPositions();
@@ -106,13 +106,24 @@ export default function SemanticMarkers({
                 lnMin += 360;
                 recentered = true;
               }
+
+              // Is the new position inside the "viewport"?
+              // (strictly this is the un-greyed-out region in the middle when zoomed well out)
+              const inBounds =
+                lng <= bounds.northEast.lng &&
+                lng >= bounds.southWest.lng &&
+                lat <= bounds.northEast.lat &&
+                lat >= bounds.southWest.lat;
+
               return recentered
                 ? cloneElement(marker, {
                     position: { lat, lng },
                     bounds: {
                       southWest: { lat: ltMin, lng: lnMin },
                       northEast: { lat: ltMax, lng: lnMax },
                     },
+                    // to prevent "fly-bys" (see #628) disable animation for out-of-bounds markers
+                    ...(inBounds ? {} : { duration: -1 }),
                   })
                 : marker;
             })
@@ -126,27 +137,23 @@ export default function SemanticMarkers({
       ) {
         // get the position-modified markers from `animationFunction`
         // see geohash.tsx for example
-        const animationValues = animation.animationFunction({
-          prevMarkers: prevRecenteredMarkers,
-          markers: recenteredMarkers,
-        });
+        const { markers: oldAndNewRepositionedMarkers } =
+          animation.animationFunction({
+            prevMarkers: prevRecenteredMarkers,
+            markers: recenteredMarkers,
+          });
         // set them as current
         // any marker that already existed will move to the modified position
         if (
           !isEqual(
-            animationValues.markers.map(({ props }) => props),
+            oldAndNewRepositionedMarkers.map(({ props }) => props),
             consolidatedMarkers.map(({ props }) => props)
           )
         )
-          setConsolidatedMarkers(animationValues.markers);
-        // then set a timer to remove the old markers when zooming out
-        // or if zooming in, switch to just the new markers straight away
-        // (their starting position was set by `animationFunction`)
-        // It's complicated but it works!
-        timeoutVariable = enqueueZoom(
-          animationValues.zoomType,
-          recenteredMarkers
-        );
+          setConsolidatedMarkers(oldAndNewRepositionedMarkers);
+
+        // we used to set a timer to remove the old markers when zooming out
+        // but now we just let the next render cycle do it.
       } else {
         /** First render of markers **/
         if (
@@ -171,27 +178,6 @@ export default function SemanticMarkers({
       )
         setPrevRecenteredMarkers(recenteredMarkers);
     }
-
-    function enqueueZoom(
-      zoomType: string | null,
-      nextMarkers: ReactElement<BoundsDriftMarkerProps>[]
-    ) {
-      /** If we are zooming in then reset the marker elements. When initially rendered
-       * the new markers will start at the matching existing marker's location and here we will
-       * reset marker elements so they will animated to their final position
-       **/
-      if (zoomType === 'in') {
-        setConsolidatedMarkers(nextMarkers);
-      } else if (zoomType === 'out') {
-        /** If we are zooming out then remove the old markers after they finish animating. **/
-        return window.setTimeout(
-          () => {
-            setConsolidatedMarkers(nextMarkers);
-          },
-          animation ? animation.duration : 0
-        );
-      }
-    }
   }, [
     animation,
     map,

packages/libs/components/src/map/animation_functions/geohash.tsx

@@ -11,8 +11,8 @@ export default function geohashAnimation({
   prevMarkers,
   markers,
 }: geoHashAnimation) {
-  const prevGeoHash = prevMarkers[0].key as string;
-  const currentGeohash = markers[0].key as string;
+  const prevGeoHash = prevMarkers[0].props.id as string;
+  const currentGeohash = markers[0].props.id as string;
   let zoomType, consolidatedMarkers;
 
   /** Zoom Out - Move existing markers to new position
@@ -22,9 +22,9 @@ export default function geohashAnimation({
   if (prevGeoHash.length > currentGeohash.length) {
     zoomType = 'out';
     const hashDif = prevGeoHash.length - currentGeohash.length;
-    // Get a new array of existing markers with new position property
+    // Get array of old markers with new positions
     const cloneArray = updateMarkers(prevMarkers, markers, hashDif);
-    // Combine the new and existing markers
+    // Combine the new and old markers
     consolidatedMarkers = [...markers, ...cloneArray];
   } else if (prevGeoHash.length < currentGeohash.length) {
     /** Zoom In - New markers start at old position
@@ -33,10 +33,12 @@ export default function geohashAnimation({
      **/
     zoomType = 'in';
     const hashDif = currentGeohash.length - prevGeoHash.length;
-    // Get a new array of new markers with existing position property
-    // Set final render markers to the cloneArray which holds the new markers with
-    // their new starting location
-    consolidatedMarkers = updateMarkers(markers, prevMarkers, hashDif);
+    // Get array of new markers with old positions
+    const cloneArray = updateMarkers(markers, prevMarkers, hashDif);
+    // put them first - ideally underneath the old markers
+    // (though it seems impossible to get this to work)
+    // ((see also updateMarkers zIndexOffset failed attempt))
+    consolidatedMarkers = [...cloneArray, ...prevMarkers];
   } else {
     /** No difference in geohashes - Render markers as they are **/
     zoomType = null;

packages/libs/components/src/map/animation_functions/updateMarkers.tsx

@@ -8,12 +8,12 @@ export default function updateMarkers(
 ) {
   return toChangeMarkers.map((markerObj) => {
     // Calculate the matching geohash
-    const sourceKey = markerObj.key as string;
+    const sourceKey = markerObj.props.id as string;
     const sourceHash = sourceKey.slice(0, -hashDif);
 
     // Find the object with the matching geohash
     const matchingMarkers = sourceMarkers.filter((obj) => {
-      return obj.key === sourceHash;
+      return obj.props.id === sourceHash;
     });
 
     // Clone marker element with new position
@@ -22,6 +22,10 @@ export default function updateMarkers(
       // Clone marker element with new position
       markerCloneProps = {
         position: matchingMarkers[0].props.position,
+        icon: {
+          ...markerObj.props.icon,
+          className: 'bottom-marker', // doesn't seem to work :-(
+        },
       };
     }
 

packages/libs/components/src/map/styles/map-styles.css

@@ -71,6 +71,10 @@
   z-index: 1;
 }
 
+.bottom-marker {
+  z-index: -1000;
+}
+
 /* Replace leaflet styles*/
 
 /* general typography */

packages/libs/coreui/src/components/buttons/PopoverButton/PopoverButton.tsx

@@ -150,6 +150,7 @@ export default function PopoverButton(props: PopoverButtonProps) {
       additionalAriaProperties={{
         'aria-controls': 'dropdown',
         'aria-haspopup': 'true',
+        type: 'button',
       }}
     />
   );

packages/libs/coreui/src/components/inputs/SelectTree/SelectTree.tsx

@@ -84,7 +84,15 @@ function SelectTree<T>(props: SelectTreeProps<T>) {
       onClose={onClose}
       isDisabled={props.isDisabled}
     >
-      {wrapPopover ? wrapPopover(checkboxTree) : checkboxTree}
+      <div
+        style={{
+          padding: '1em 1em 1em .5em',
+          width: '30em',
+          height: 'min(60vh, 40em)',
+        }}
+      >
+        {wrapPopover ? wrapPopover(checkboxTree) : checkboxTree}
+      </div>
     </PopoverButton>
   );
 }

packages/libs/eda/src/lib/core/components/variableSelectors/VariableList.tsx

@@ -870,9 +870,6 @@ export default function VariableList({
             style={{
               position: 'relative',
               borderRadius: '0.25em',
-              padding: '0.5em 0.5em 0.5em 0',
-              height: '60vh',
-              width: '30em',
               overflow: 'hidden',
               display: 'flex',
               flexDirection: 'column',

packages/libs/eda/src/lib/core/components/visualizations/InputVariables.tsx

@@ -10,13 +10,17 @@ import {
 
 import VariableTreeDropdown from '../variableSelectors/VariableTreeDropdown';
 import { Toggle } from '@veupathdb/coreui';
-import { makeEntityDisplayName } from '../../utils/study-metadata';
+import {
+  findEntityAndVariable,
+  makeEntityDisplayName,
+} from '../../utils/study-metadata';
 import { useInputStyles } from './inputStyles';
 import { Tooltip } from '@veupathdb/coreui';
 import RadioButtonGroup from '@veupathdb/components/lib/components/widgets/RadioButtonGroup';
 import { isEqual } from 'lodash';
 import { red } from '@veupathdb/coreui/lib/definitions/colors';
 import { CSSProperties } from '@material-ui/core/styles/withStyles';
+import Banner from '@veupathdb/coreui/lib/components/banners/Banner';
 
 export interface InputSpec {
   name: string;
@@ -182,12 +186,27 @@ export function InputVariables(props: Props) {
     onChange({ ...selectedVariables, [inputName]: selectedVariable });
   };
 
+  const invalidInputs = inputs.filter((inputSpec) => {
+    const variableDescriptor = selectedVariables[inputSpec.name];
+    if (variableDescriptor == null) return false;
+    const entityAndVariable = findEntityAndVariable(
+      entities,
+      variableDescriptor
+    );
+    return (
+      entityAndVariable == null ||
+      entityAndVariable.variable.type === 'category'
+    );
+  });
+
   // Find entities that are excluded for each variable, and union their variables
   // with the disabled variables.
   const disabledVariablesByInputName: Record<string, VariableDescriptor[]> =
     useMemo(
       () =>
         inputs.reduce((map, input) => {
+          // ignore invalid inputs
+          if (invalidInputs.includes(input)) return map;
           // For each input (ex. xAxisVariable), determine its constraints based on which patterns any other selected variables match.
           const filteredConstraints =
             constraints &&
@@ -210,6 +229,7 @@ export function InputVariables(props: Props) {
         }, {} as Record<string, VariableDescriptor[]>),
       [
         inputs,
+        invalidInputs,
         constraints,
         selectedVariables,
         entities,
@@ -270,7 +290,8 @@ export function InputVariables(props: Props) {
                           constraints &&
                           constraints.length &&
                           constraints[0][input.name]?.isRequired &&
-                          !selectedVariables[input.name]
+                          (!selectedVariables[input.name] ||
+                            invalidInputs.includes(input))
                             ? requiredInputLabelStyle
                             : input.role === 'stratification' &&
                               hasMultipleStratificationValues
@@ -351,8 +372,16 @@ export function InputVariables(props: Props) {
                         }
                         starredVariables={starredVariables}
                         toggleStarredVariable={toggleStarredVariable}
-                        entityId={selectedVariables[input.name]?.entityId}
-                        variableId={selectedVariables[input.name]?.variableId}
+                        entityId={
+                          invalidInputs.includes(input)
+                            ? undefined
+                            : selectedVariables[input.name]?.entityId
+                        }
+                        variableId={
+                          invalidInputs.includes(input)
+                            ? undefined
+                            : selectedVariables[input.name]?.variableId
+                        }
                         variableLinkConfig={{
                           type: 'button',
                           onClick: (variable) =>
@@ -396,6 +425,30 @@ export function InputVariables(props: Props) {
           {content}
         </div>
       ))}
+      {invalidInputs.length > 0 && (
+        <Banner
+          banner={{
+            type: 'error',
+            message: (
+              <div>
+                The following inputs reference a variable that no longer exists.
+                Use the dropdown to choose a new variable.
+                {
+                  <ul>
+                    {invalidInputs.map((inputSpec) => {
+                      return (
+                        <li>
+                          <strong>{inputSpec.label}</strong>
+                        </li>
+                      );
+                    })}
+                  </ul>
+                }
+              </div>
+            ),
+          }}
+        />
+      )}
     </div>
   );
 }

packages/libs/eda/src/lib/core/utils/data-element-constraints.ts

@@ -286,6 +286,13 @@ export function filterConstraints(
       if (value == null) return true;
       // Ignore constraints that are on this selectedVarReference
       if (selectedVarReference === variableName) return true;
+      // {{ treat invalid selections as "empty" and return true
+      const entityAndVariable = findEntityAndVariable(entities, value);
+      if (entityAndVariable == null) return true;
+      const { variable } = entityAndVariable;
+      if (variable.type === 'category') return true;
+      // }}
+
       // If a constraint does not declare shapes or types and it allows multivalued variables, then any value is allowed, thus the constraint is "in-play"
       if (
         isEmpty(constraint.allowedShapes) &&
@@ -296,15 +303,8 @@ export function filterConstraints(
         constraint.allowMultiValued
       )
         return true;
+
       // Check that the value's associated variable has compatible characteristics
-      const entityAndVariable = findEntityAndVariable(entities, value);
-      if (entityAndVariable == null)
-        throw new Error(
-          `Could not find selected entity and variable: entityId = ${value.entityId}; variableId = ${value.variableId}.`
-        );
-      const { variable } = entityAndVariable;
-      if (variable.type === 'category')
-        throw new Error('Categories are not allowed for variable constraints.');
       const typeIsValid =
         isEmpty(constraint.allowedTypes) ||
         constraint.allowedTypes?.includes(variable.type);