Skip to content

Commit

Permalink
fix: LEAP-1579: LEAP-1576: LEAP-1582: Improve comment icons usability (
Browse files Browse the repository at this point in the history
…#6546)

Co-authored-by: Gondragos <[email protected]>
  • Loading branch information
Gondragos and Gondragos authored Oct 24, 2024
1 parent 884be70 commit 094e5d7
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ type CommentItemProps = {
const CommentItem: React.FC<CommentItemProps> = observer(({ comment, rootRef }) => {
const root = rootRef.current;
const node = comment.regionRef?.overlayNode;
// result is hidden when it's per-region and the region is not selected
const isHiddenResult = node?.area && !node.area.selected && !node.area.classification;
const isHidden = !node || node.hidden || isHiddenResult;
const isHidden = !node;
// {} !== {} it's always so, and it's a way to force re-render
const [forceUpdateId, forceUpdate] = useState<any>({});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ const parentImagePropsWatch = {
};

const obtainWatcher = (node) => {
// that's a tricky way to get watcher also for an exact result instead of whole region
// works for global classifications and per-regions
const isResult = !!node.from_name;
if (isResult) {
return DOMWatcher;
}

switch (node.type) {
case "richtextregion":
case "paragraphs":
Expand Down
10 changes: 10 additions & 0 deletions web/libs/editor/src/regions/Result.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,16 @@ const Result = types

return { strokecolor, strokewidth, fillcolor, fillopacity, opacity };
},

/**
* This name historically is used for the region elements for getting their bboxes.
* Now we need it for a result also.
* Let's say "Region" here means just an area on the screen.
* So that it's an element through which we can get the bbox for an area where classification takes place.
*/
getRegionElement() {
return self.from_name?.getRegionElement?.();
},
}))
.volatile(() => ({
pid: "",
Expand Down
35 changes: 33 additions & 2 deletions web/libs/editor/src/stores/Comment/Anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ export const Anchor = types
controlName: types.maybe(types.string),
})
.views((self) => ({
get comment() {
return getParent(self);
},
get annotation() {
return getParent(self).annotation;
return self.comment.annotation;
},
get region() {
return self.annotation.regions.find((r) => r.cleanId === self.regionId);
Expand All @@ -27,7 +30,35 @@ export const Anchor = types
* @return {Object} The overlays-applicable node of the anchor.
*/
get overlayNode() {
return self.result ?? self.region;
const { result, region } = self;
if (self.comment.isResolved || self.comment.isDeleted) return null;
if (!region || region.hidden) return null;
const isOnCurrentItem = (region.item_index ?? 0) === (region.object.currentItemIndex ?? 0);
if (!isOnCurrentItem) return null;

if (result) {
const controlTag = result.from_name;
// Most probably, it's always true as we should work only with classification results for now.
// If this is not the case, then at the time of writing this comment,
// we assume that we have no way of displaying anything related to the overlay for this result.
const isClassification = controlTag.isClassificationTag;
// Taking into account `visiblewhen`
const isVisible = controlTag.isVisible !== false;
// The result that is displayed at the control tag right now
const currentResult = controlTag.result;
// It'll always be true for perObject mode,
// and for perRegion/perItem it'll be true only if the result is already displayed at the control tag
// (related region/item are selected)
const isCurrentResult = currentResult === result;
const isDisplayedAtControlTag = isClassification && isVisible && isCurrentResult;
if (isDisplayedAtControlTag) {
return result;
}
}

// if a result does not exist,
// or it's hidden, we still need to indicate comment existence on its region if it's possible
return self.region;
},
/**
* A key that should be unique in the context of the current annotation and current moment
Expand Down
9 changes: 9 additions & 0 deletions web/libs/editor/src/tags/control/ClassificationBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ const ClassificationBase = types
}
return self.annotation.results.find((r) => r.from_name === self);
},

/**
* That name historically was used for regions, but now it's used for the classifications as well.
* Let's say "Region" here means just an area on the screen.
* So that it's an element through which we can get the bbox For an area where classification takes place.
*/
getRegionElement() {
return self.elementRef.current;
},
};
})
.actions((self) => {
Expand Down

0 comments on commit 094e5d7

Please sign in to comment.