frontend/javascripts/oxalis/model/sagas/volume/floodfill_saga.tsx (1)

35-102: Ensure proper handling of failure reasons in getBoundingBoxForFloodFill

In the getBoundingBoxForFloodFill function, when returning failure reasons, ensure that the failure messages are user-friendly and informative. This will enhance the user's understanding when the flood fill operation cannot proceed.

frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration.spec.ts (1)

Line range hint 330-347: Improve test reliability with explicit undo actions

When performing undo and redo operations in the test, ensure that each action is awaited properly to prevent race conditions and enhance test stability.

Apply this diff to await undo and redo actions explicitly:

- await dispatchUndoAsync(Store.dispatch);
+ await dispatchUndoAsync(Store.dispatch);
+ await t.context.api.tracing.save();

- await dispatchRedoAsync(Store.dispatch);
+ await dispatchRedoAsync(Store.dispatch);
+ await t.context.api.tracing.save();

frontend/stylesheets/trace_view/_action_bar.less (1)

71-74: Consider using a more semantic class name

The class name .undo-redo-button suggests specific usage, but the functionality (preventing width flickering) could be useful for other buttons. Consider a more generic name like .fixed-width-button or .no-flicker-button.

CHANGELOG.unreleased.md (1)

16-16: Enhance the changelog entry with more details

Consider expanding the changelog entry to provide more comprehensive information about the feature:

-The fill tool can now be adapted so that it only acts within a specified bounding box. Use the new "Restrict Floodfill" mode for that in the toolbar. [#8267](https://github.com/scalableminds/webknossos/pull/8267)
+The fill tool can now be restricted to operate within a specified bounding box. Enable the new "Restrict Floodfill" toggle in the toolbar when the fill tool is selected. If no bounding box exists when using this mode, a notification will be displayed. This feature helps prevent unintended fill operations outside the area of interest. [#8267](https://github.com/scalableminds/webknossos/pull/8267)

frontend/javascripts/oxalis/default_state.ts (1)

87-87: LGTM! Consider adding JSDoc

The new configuration property is well-named and appropriately placed. Consider adding JSDoc documentation to describe its purpose and behavior:

+    /** When enabled, restricts flood fill operations to the current bounding box */
     isFloodfillRestrictedToBoundingBox: false,

frontend/javascripts/oxalis/constants.ts (1)

336-338: Consider adding more detailed documentation.

While the comment explains the purpose, it would be helpful to document:

1. Why the value of 10 was chosen
2. What "more lax" means in terms of actual behavior
3. The impact on performance and memory usage

frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts (2)

501-509: Consider adding more detailed documentation for the dynamic bounding box expansion

While the implementation is solid, the comment could be more explicit about:

• The purpose of allowing multiple additions of the same bucket
• The implications of the "neighbour volume shape" concept
• How this relates to the new bounding box restriction feature

---

695-724: Consider enhancing the progress logging

The progress logging implementation could be improved:

1. Consider using a constant for the progress update interval (1000000)
2. Consider adding an estimate of total progress (e.g., percentage complete)
3. Consider adding more context about the operation in the progress message

-                if (labeledVoxelCount % 1000000 === 0) {
-                  console.log(`Labeled ${labeledVoxelCount} Vx. Continuing...`);
-
-                  await progressCallback(
-                    false,
-                    `Labeled ${labeledVoxelCount / 1000000} MVx. Continuing...`,
-                  );
+                const PROGRESS_UPDATE_INTERVAL = 1000000;
+                if (labeledVoxelCount % PROGRESS_UPDATE_INTERVAL === 0) {
+                  const message = `Flood fill: Labeled ${(labeledVoxelCount / 1000000).toFixed(1)} MVx`;
+                  console.log(message);
+                  await progressCallback(false, message);

frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx (1)

1372-1405: Consider enhancing the FloodFillSettings component

While the implementation is solid, consider these improvements:

1. Extract the tooltip text to a constant for reusability
2. Add a data-testid attribute for easier testing
3. Consider using a memo hook for the toggle callback to prevent unnecessary re-renders

+const FLOOD_FILL_BBOX_TOOLTIP = 
+  "When enabled, the floodfill will be restricted to the bounding box enclosed by the clicked position. If multiple bounding boxes enclose that position, the smallest is used.";

 function FloodFillSettings() {
   const dispatch = useDispatch();
   const isRestrictedToBoundingBox = useSelector(
     (state: OxalisState) => state.userConfiguration.isFloodfillRestrictedToBoundingBox,
   );
-  const toggleRestrictFloodfillToBoundingBox = () => {
+  const toggleRestrictFloodfillToBoundingBox = useCallback(() => {
     dispatch(
       updateUserSettingAction("isFloodfillRestrictedToBoundingBox", !isRestrictedToBoundingBox),
     );
-  };
+  }, [dispatch, isRestrictedToBoundingBox]);

   return (
-    <div>
+    <div data-testid="flood-fill-settings">
       <FillModeSwitch />

       <ButtonComponent
         style={{
           opacity: isRestrictedToBoundingBox ? 1 : 0.5,
           marginLeft: 12,
         }}
         type={isRestrictedToBoundingBox ? "primary" : "default"}
         onClick={toggleRestrictFloodfillToBoundingBox}
-        title={
-          "When enabled, the floodfill will be restricted to the bounding box enclosed by the clicked position. If multiple bounding boxes enclose that position, the smallest is used."
-        }
+        title={FLOOD_FILL_BBOX_TOOLTIP}
       >

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx:433-434
Timestamp: 2024-11-22T17:19:07.947Z
Learning: In the codebase, certain usages of `segmentationLayer.resolutions` are intentionally retained and should not be changed to `segmentationLayer.mags` during refactoring.

 export const getUserBoundingBoxesThatContainPosition = (
   state: OxalisState,
   position: Vector3,
 ): Array<UserBoundingBox> => {
   const bboxes = getUserBoundingBoxesFromState(state);
+  const boundingBoxInstances = bboxes.map(
+    (el) => new BoundingBox(el.boundingBox)
+  );
 
-  return bboxes.filter((el) => new BoundingBox(el.boundingBox).containsPoint(position));
+  return bboxes.filter((_, idx) => 
+    boundingBoxInstances[idx].containsPoint(position)
+  );
 };