Skip to content

1411 encounter annotation zindex#1454

Open
erinz2020 wants to merge 2 commits intomainfrom
1411_encounter_annotation_zindex
Open

1411 encounter annotation zindex#1454
erinz2020 wants to merge 2 commits intomainfrom
1411_encounter_annotation_zindex

Conversation

@erinz2020
Copy link
Contributor

@erinz2020 erinz2020 commented Feb 13, 2026

on encounter page -> image card and full screen image, small annotations can be covered by large annotations and thus not clickable.
this pr addresses this issue by calculating annotations area, giving smaller annotation higher zindex, clicked annotation has highest zindex, so the icons won't be covered by other annotations.

PR fixes #1411

@erinz2020 erinz2020 requested a review from naknomum February 13, 2026 18:19
@erinz2020 erinz2020 marked this pull request as ready for review February 13, 2026 18:19
Copy link
Member

@naknomum naknomum left a comment

Choose a reason for hiding this comment

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

nice!

@erinz2020
Copy link
Contributor Author

@claude review this PR

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 13, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 14, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 14, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 14, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 14, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@erinz2020
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Feb 14, 2026

Claude finished @erinz2020's task in 1m 0s —— View job


PR Review

Tasks

  • Read repository guidelines (CLAUDE.md)
  • Review changed files
  • Analyze z-index implementation
  • Provide review feedback

Summary

This PR successfully addresses issue #1411 by implementing a smart z-index system for annotations. The solution calculates annotation areas and assigns higher z-index values to smaller annotations, ensuring they remain clickable even when overlapping with larger ones. The clicked annotation gets the highest z-index (2000).

Implementation Review

Core Logic (both files use identical approach):

The z-index calculation in both ImageModal.jsx:398-404 and ImageCard.jsx:280-284 follows this pattern:

const area = (rect.width || 0) * (rect.height || 0);
const score = 1 - area / maxArea;
const baseZ = 10 + Math.round(score * 1000);
const finalZ = rect.annotationId === selectedAnnotationId ? 2000 : baseZ;

Strengths:

  • ✅ Simple and effective algorithm - smaller annotations get higher z-index
  • ✅ Consistent implementation across both ImageCard and ImageModal
  • ✅ Clicked/selected annotations always appear on top (z-index 2000)
  • ✅ Good range separation (10-1010 for base, 2000 for selected)
  • ✅ Uses useMemo to optimize maxArea calculation

Potential Issues:

  1. Division by zero protection: The code uses maxArea as denominator without checking if it could be zero. The maxArea is initialized to 1 in the reduce (lines 154-158 in ImageModal, 200-205 in ImageCard), which protects against division by zero. ✅ Good

  2. Edge case - single annotation: When there's only one annotation, area === maxArea, so score = 0, resulting in baseZ = 10. This works fine. ✅

  3. Edge case - all same size: If all annotations are the same size, they all get the same z-index. The order would depend on render order. This is acceptable behavior. ✅

  4. Z-index value range: Base z-index ranges from 10 (largest) to 1010 (smallest). This provides excellent separation and won't conflict with typical UI z-indexes. ✅

Code Quality

ImageModal.jsx:153-158

const maxArea = React.useMemo(() => {
  return rects.reduce(
    (max, r) => Math.max(max, (r.width || 0) * (r.height || 0)),
    1,
  );
}, [rects]);

ImageCard.jsx:200-205

const maxArea = React.useMemo(() => {
  return rects.reduce(
    (max, r) => Math.max(max, (r.width || 0) * (r.height || 0)),
    1,
  );
}, [rects]);

Both implementations properly use useMemo with rects as the dependency, ensuring efficient recalculation only when annotations change.

Recommendation

✅ Approve - The implementation is solid, well-thought-out, and solves the stated problem effectively. The code is consistent across both components, properly handles edge cases, and uses React best practices with useMemo for performance optimization.

The z-index algorithm is elegant: it creates a clear visual hierarchy where smaller (presumably more important) annotations remain accessible, while still allowing users to bring any annotation to the front by clicking on it.

--- | Branch: 1411_encounter_annotation_zindex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

encounter page image/annotation display, larger annotations can obscure smaller annotation actions

2 participants