[OSDEV-2374] Implement claim banner with status and cta support#890
Conversation
React App | Jest test suite - Code coverage reportTotal: 41.22%Your code coverage diff: 0.68% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional interactive DialogTooltip (with learn-more link), refactors ClaimFlag into a Grid-based component that surfaces claimInfo and formatted dates, updates related styles and utilities (typography + date formatting), propagates claim/embed state into ProductionLocationDetailsContent, adjusts ClosureStatus layout/text, and adds unit tests and a release note. Changes
Sequence Diagram(s)(Skipped — UI/component updates only; no multi-system sequential flow rendered.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx (1)
235-241: Includeapproved_atinclaimInfoPropTypes.
approved_atis used at runtime but not declared in the prop contract, which weakens payload validation/documentation.♻️ Proposed fix
claimInfo: PropTypes.shape({ contributor: PropTypes.oneOfType([ PropTypes.string, PropTypes.shape({ name: PropTypes.string }), ]), + approved_at: PropTypes.string, created_at: PropTypes.string, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx` around lines 235 - 241, The PropTypes for claimInfo in the ClaimFlag component are missing the approved_at field used at runtime; update the PropTypes.shape for claimInfo in ClaimFlag.jsx to include approved_at (e.g., approved_at: PropTypes.string) so the prop contract validates/documentates this timestamp field alongside contributor and created_at.src/react/src/components/ProductionLocation/Heading/ClaimFlag/styles.js (1)
5-31: Use the computedspacingtoken consistently.
spacinghas a fallback, butpaddingRightstill usestheme.spacing.unitdirectly. Usespacingthere too to keep fallback behavior coherent.♻️ Proposed fix
- paddingRight: theme.spacing.unit * 1.5, + paddingRight: spacing * 1.5,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/styles.js` around lines 5 - 31, The computed spacing fallback variable is declared as spacing but paddingRight in the iconColumn still uses theme.spacing.unit directly; update paddingRight to use the spacing variable (e.g., spacing * 1.5) so the fallback is applied consistently, and scan other style props in the returned object (root, row, iconColumn) to replace any remaining theme.spacing.unit uses with spacing; reference the spacing const and the iconColumn object in this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/src/components/Contribute/DialogTooltip.jsx`:
- Around line 1-38: A pending interactive close timeout (stored in
closeTimerRef) can fire after the component unmounts and call setOpen(false);
fix this by ensuring any scheduled timeout is cleared before setting a new one
and on unmount: update handleTriggerLeave to call clearCloseTimer() before
assigning closeTimerRef.current, and add a useEffect cleanup that calls
clearCloseTimer() when the component unmounts; reference closeTimerRef,
clearCloseTimer, handleTriggerLeave and setOpen so the timeout is always cleared
to prevent stale setOpen calls.
- Around line 94-113: When interactive is true, the trigger wrapper only reacts
to mouse events so keyboard users cannot open the Tooltip; update the trigger
wrapper (the span around childComponent used in triggerWrapper) to be
keyboard-accessible by adding tabIndex={0}, onFocus={() => setOpen(true)},
onBlur={() => setOpen(false)} (and optionally onKeyDown to open on Enter/Space)
alongside the existing handleTriggerEnter/handleTriggerLeave, and ensure the
controlled open prop (open) and onClose handler on Tooltip remain consistent;
also add an aria-describedby (or id) connection between the trigger and Tooltip
titleContent so assistive tech can associate them.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Around line 107-110: The logic for computing isClaimed is wrong: update the
checks (in ProductionLocationDetailsContent.jsx's isPendingClaim/isClaimed and
the analogous code in FacilityDetailsContent.jsx) so that isClaimed is true only
when data?.properties?.claim_info?.status ===
facilityClaimStatusChoicesEnum.APPROVED (and isPendingClaim remains the PENDING
check); replace the current negation-based check (!!data?.properties?.claim_info
&& !isPendingClaim) with an explicit status equality to
facilityClaimStatusChoicesEnum.APPROVED to avoid treating DENIED or REVOKED as
claimed.
---
Nitpick comments:
In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx`:
- Around line 235-241: The PropTypes for claimInfo in the ClaimFlag component
are missing the approved_at field used at runtime; update the PropTypes.shape
for claimInfo in ClaimFlag.jsx to include approved_at (e.g., approved_at:
PropTypes.string) so the prop contract validates/documentates this timestamp
field alongside contributor and created_at.
In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/styles.js`:
- Around line 5-31: The computed spacing fallback variable is declared as
spacing but paddingRight in the iconColumn still uses theme.spacing.unit
directly; update paddingRight to use the spacing variable (e.g., spacing * 1.5)
so the fallback is applied consistently, and scan other style props in the
returned object (root, row, iconColumn) to replace any remaining
theme.spacing.unit uses with spacing; reference the spacing const and the
iconColumn object in this change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/react/src/components/Contribute/DialogTooltip.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/styles.jssrc/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/util/typographyStyles.js
...nts/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx
Show resolved
Hide resolved
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/react/src/components/Contribute/DialogTooltip.jsx (1)
75-99: Factor duplicatedpopperOptionsinto a single base object.Both branches repeat the same arrow modifier config; extracting it will reduce noise and future drift risk.
♻️ Proposed refactor
+ const basePopperProps = { + popperOptions: { + modifiers: { + arrow: { + enabled: Boolean(arrowRef), + element: arrowRef, + }, + }, + }, + }; + const popperProps = interactive ? { onMouseEnter: handlePopperEnter, onMouseLeave: handlePopperLeave, onFocus: handlePopperEnter, onBlur: handlePopperLeave, - popperOptions: { - modifiers: { - arrow: { - enabled: Boolean(arrowRef), - element: arrowRef, - }, - }, - }, + ...basePopperProps, } - : { - popperOptions: { - modifiers: { - arrow: { - enabled: Boolean(arrowRef), - element: arrowRef, - }, - }, - }, - }; + : basePopperProps;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/Contribute/DialogTooltip.jsx` around lines 75 - 99, The popperOptions block is duplicated in the construction of popperProps; extract a single base object (e.g., const basePopperOptions = { popperOptions: { modifiers: { arrow: { enabled: Boolean(arrowRef), element: arrowRef } } } };) and then create popperProps by spreading basePopperOptions into both branches, adding the interactive handlers only when interactive is true (use handlePopperEnter and handlePopperLeave for onMouseEnter/onMouseLeave/onFocus/onBlur). Ensure references to popperProps and arrowRef remain unchanged.src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx (1)
153-217: Consider extracting claimed-by text composition into a small helper.This conditional block is correct but hard to scan; moving it into a renderer/helper would improve readability and testability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx` around lines 153 - 217, Extract the JSX that composes the claimed-by text into a small helper inside the ClaimFlag component (e.g., a function named renderClaimedByText or getClaimedByNode) that accepts contributorName and formattedDate and returns the appropriate JSX node using classes.subtitleSameLine and classes.inlineHighlight; replace the large conditional block inside the showClaimedByLine Typography with a single call to this helper, and ensure the helper is exported or unit-testable if needed for test coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/react/src/components/Contribute/DialogTooltip.jsx`:
- Around line 75-99: The popperOptions block is duplicated in the construction
of popperProps; extract a single base object (e.g., const basePopperOptions = {
popperOptions: { modifiers: { arrow: { enabled: Boolean(arrowRef), element:
arrowRef } } } };) and then create popperProps by spreading basePopperOptions
into both branches, adding the interactive handlers only when interactive is
true (use handlePopperEnter and handlePopperLeave for
onMouseEnter/onMouseLeave/onFocus/onBlur). Ensure references to popperProps and
arrowRef remain unchanged.
In `@src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx`:
- Around line 153-217: Extract the JSX that composes the claimed-by text into a
small helper inside the ClaimFlag component (e.g., a function named
renderClaimedByText or getClaimedByNode) that accepts contributorName and
formattedDate and returns the appropriate JSX node using
classes.subtitleSameLine and classes.inlineHighlight; replace the large
conditional block inside the showClaimedByLine Typography with a single call to
this helper, and ensure the helper is exported or unit-testable if needed for
test coverage.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/react/src/__tests__/components/ProductionLocationClosureStatus.test.jsxsrc/react/src/components/Contribute/DialogTooltip.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsxsrc/react/src/components/ProductionLocation/Heading/ClosureStatus/ClosureStatus.jsx
✅ Files skipped from review due to trivial changes (1)
- src/react/src/tests/components/ProductionLocationClosureStatus.test.jsx
protsack-stephan
left a comment
There was a problem hiding this comment.
Great work! Couple of structural comments from me!
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/utils.js
Outdated
Show resolved
Hide resolved
…s-and-CTA-support
protsack-stephan
left a comment
There was a problem hiding this comment.
Good work! Couple of comments from me, it's up to you to decide whether to implement them or leave them as is.
...ct/src/components/ProductionLocation/ProductionLocationDetails/ProductionLocationDetails.jsx
Show resolved
Hide resolved
...ct/src/components/ProductionLocation/ProductionLocationDetails/ProductionLocationDetails.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/ProductionLocation/Heading/ClaimFlag/ClaimFlag.jsx
Outdated
Show resolved
Hide resolved
|



Implements OSDEV-2374
Created UI for Claim and Closure status banners.
Additional:
<DialogTooltip/>component so that it does not close the popup on mouseover. This change allows users to access theLearn more →link.isEmbedprop to the<ClaimFlag/>component. Although we redirect to the legacy facility page in embed mode, this serves as an additional guard to prevent claiming locations from embed mode.getTypographyStylesto make sure common style rules will be reused across other components.