Conversation
…xt to FacetListItem
WalkthroughAdds propagation of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as RelatedContent / SearchBox
participant Listing as WorksListing
participant Facets as WorkFacets / FacetList
participant Backend as Query Layer
User->>UI: open page or type in SearchBox
UI->>Listing: navigate/update list URL (base url + '?' + params including client-id / organization-relation-type)
Listing->>Backend: request works with mapped variables
Backend->>Backend: mapSearchParams -> include clientId, organizationRelationType
Backend-->>Listing: return QueryData (works, clients, facets with tooltipText)
Listing->>Facets: render WorkFacets (includes client-facets and searchBox)
Facets->>User: render facet items with InfoTooltip where present
User->>Facets: select facet
Facets->>Listing: trigger filtered fetch
Listing->>Backend: fetch filtered works
Backend-->>Listing: return filtered works
Listing->>User: display filtered results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/FacetList/FacetList.tsx (1)
38-48:⚠️ Potential issue | 🟡 MinorMinor typo in React key.
Line 40 has an extra space in the key:
'facet-item- ' +idshould be'facet-item-' + id. While this doesn't break functionality, it creates inconsistent key formatting.Proposed fix
<FacetListItem - key={'facet-item- ' +id + '-' + i} + key={'facet-item-' + id + '-' + i} facet={facet}
🤖 Fix all issues with AI agents
In `@src/app/`(main)/doi.org/[...doi]/RelatedContent.tsx:
- Line 15: Remove the unused import SummarySearchMetrics from the RelatedContent
component: locate the import statement "import SummarySearchMetrics from
'src/components/SummarySearchMetrics/SummarySearchMetrics'" and delete it (or
remove SummarySearchMetrics from the import list if combined) so the file no
longer imports an identifier that isn't referenced anywhere in
RelatedContent.tsx.
In `@src/app/`(main)/doi.org/page.tsx:
- Around line 24-25: The SearchParams interface is missing hyphenated keys
referenced in page.tsx (you access vars['repository-type'] and vars['client-id']
and earlier vars['resource-type'], vars['field-of-science'],
vars['registration-agency']); add these hyphenated properties to the
SearchParams type (e.g., as optional entries with the same union type used for
other params, such as string | string[] | undefined) so clientType and clientId
assignments are properly typed and consistent with the existing 'resource-type',
'field-of-science', and 'registration-agency' additions.
In `@src/app/`(main)/repositories/[...repoid]/RelatedContent.tsx:
- Around line 56-61: The JSX block in the RelatedContent component has
inconsistent indentation: the opening <Col md={3} className="d-none d-md-block">
line (containing SearchBox) is over-indented compared to surrounding lines; fix
by reducing the leading spaces so the <Col md={3} ...> line and its nested
<SearchBox path={url} /> align with the other sibling <Col md={9}> block and the
file's usual 8-space indenting style (locate this block around the
RelatedContent component and adjust the whitespace only).
In `@src/components/InfoTooltip/InfoTooltip.tsx`:
- Around line 11-27: The InfoTooltip component uses a static Tooltip id
("infoTooltip") and a span with role="button"; replace the static id by calling
React.useId() inside the InfoTooltip component (e.g., const id = React.useId())
and pass that id to the Tooltip (id={id}) and any ARIA attributes as needed to
ensure unique, SSR-safe IDs for each instance (referencing InfoTooltip,
OverlayTrigger, Tooltip). Also remove role="button" from the span since it is
non-actionable (keep onClick stopPropagation), and ensure tabIndex/aria-label
remain appropriate for accessibility.
In `@src/components/WorkFacets/WorkFacets.tsx`:
- Around line 93-95: The current checks using url.startsWith('/repositories')
are brittle for relative or missing-leading-slash inputs; update the logic in
WorkFacets (the component/function where url is inspected) to normalize the URL
before checking the path: create a URL object with a dummy base (e.g. new
URL(url, 'http://example')) and use its pathname.startsWith('/repositories') for
the route check, and apply the same normalization for the other occurrence
mentioned (the check at the second location around line 178) so both branches
reliably detect repository routes regardless of leading slash or relative paths.
🧹 Nitpick comments (1)
src/components/FacetList/FacetListItem.tsx (1)
14-66: Avoid nesting the tooltip’s interactive element inside the Link.
InfoTooltiprenders a focusable element (role="button",tabIndex=0), so embedding it inside the anchor can confuse screen readers and keyboard navigation. Consider moving the tooltip outside the<Link>(or making it non-focusable in this context) and verify with an a11y scan.
| clientType: vars['repository-type'], | ||
| clientId: vars['client-id'] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/app/\(main\)/doi.org/page.tsxRepository: datacite/akita
Length of output: 1351
🏁 Script executed:
find src/data/queries -name "*searchDoiQuery*" -type fRepository: datacite/akita
Length of output: 92
🏁 Script executed:
cat -n src/data/queries/searchDoiQuery.tsRepository: datacite/akita
Length of output: 13533
🏁 Script executed:
cat -n tsconfig.jsonRepository: datacite/akita
Length of output: 1121
🏁 Script executed:
rg "vars\['" src/app/\(main\)/doi.org/page.tsxRepository: datacite/akita
Length of output: 272
Add hyphenated params to SearchParams for explicit typing.
vars['repository-type'] and vars['client-id'] are accessed but not declared on SearchParams, leaving these parameters untyped. Note that similar hyphenated keys ('resource-type', 'field-of-science', 'registration-agency') on lines 21-23 have the same issue. Consider adding these to the interface for consistency and type safety.
🔧 Suggested type fix
interface SearchParams extends Partial<QueryVar> {
filterQuery?: string
+ "repository-type"?: string
+ "client-id"?: string
+ "resource-type"?: string
+ "field-of-science"?: string
+ "registration-agency"?: string
}🤖 Prompt for AI Agents
In `@src/app/`(main)/doi.org/page.tsx around lines 24 - 25, The SearchParams
interface is missing hyphenated keys referenced in page.tsx (you access
vars['repository-type'] and vars['client-id'] and earlier vars['resource-type'],
vars['field-of-science'], vars['registration-agency']); add these hyphenated
properties to the SearchParams type (e.g., as optional entries with the same
union type used for other params, such as string | string[] | undefined) so
clientType and clientId assignments are properly typed and consistent with the
existing 'resource-type', 'field-of-science', and 'registration-agency'
additions.
| "repository-type-facets", | ||
| "client-facets" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/components/WorkFacets/WorkFacets.tsx | head -200Repository: datacite/akita
Length of output: 6190
🏁 Script executed:
rg -n "url\.startsWith\('/repositories" src/components/WorkFacets/WorkFacets.tsx -B3 -A3Repository: datacite/akita
Length of output: 261
🏁 Script executed:
rg -n "<WorkFacets" --type=tsx -B3 -A3Repository: datacite/akita
Length of output: 85
🏁 Script executed:
rg -n "SearchOrganization" --type=tsxRepository: datacite/akita
Length of output: 85
🏁 Script executed:
rg -n "WorkFacets" --type=ts -C3Repository: datacite/akita
Length of output: 1642
🏁 Script executed:
rg -n "SearchOrganization" --type=ts -C5Repository: datacite/akita
Length of output: 2771
🏁 Script executed:
fd -e tsx -e ts | xargs rg -l "WorkFacets" | head -20Repository: datacite/akita
Length of output: 143
🏁 Script executed:
rg -n "url=\|url =" src/components -C2 | head -100Repository: datacite/akita
Length of output: 40
🏁 Script executed:
cat src/components/WorksListing/WorksListing.tsxRepository: datacite/akita
Length of output: 3584
🏁 Script executed:
cat src/components/SearchOrganization/SearchOrganization.tsxRepository: datacite/akita
Length of output: 2891
🏁 Script executed:
rg -n "WorksListing" --type=ts -C5Repository: datacite/akita
Length of output: 11163
🏁 Script executed:
rg -n "<WorksListing" -C5Repository: datacite/akita
Length of output: 4888
🏁 Script executed:
grep -n "url=" src/app/\(main\)/repositories/\[\.\.\.\repoid\]/RelatedContent.tsx | head -20Repository: datacite/akita
Length of output: 84
🏁 Script executed:
cat src/app/\(main\)/repositories/\[\.\.\.\repoid\]/RelatedContent.tsx | sed -n '63,85p'Repository: datacite/akita
Length of output: 423
🏁 Script executed:
cat src/app/\(main\)/ror.org/\[rorid\]/RelatedContent.tsx | sed -n '72,90p'Repository: datacite/akita
Length of output: 609
🏁 Script executed:
cat src/components/SearchWork/SearchWork.tsx | sed -n '44,60p'Repository: datacite/akita
Length of output: 433
🏁 Script executed:
cat src/app/\(main\)/doi.org/\[\.\.\.\doi\]/RelatedContent.tsx | sed -n '69,85p'Repository: datacite/akita
Length of output: 683
🏁 Script executed:
cat src/app/\(main\)/repositories/\[\.\.\.\repoid\]/RelatedContent.tsx | head -60Repository: datacite/akita
Length of output: 1963
🏁 Script executed:
cat src/app/\(main\)/ror.org/\[rorid\]/RelatedContent.tsx | head -60Repository: datacite/akita
Length of output: 2052
🏁 Script executed:
cat src/app/\(main\)/doi.org/\[\.\.\.\doi\]/RelatedContent.tsx | head -60Repository: datacite/akita
Length of output: 2226
Strengthen URL normalization when checking for repository routes.
The current logic url.startsWith('/repositories') assumes all URLs have a leading slash. While all current callers properly format URLs with leading slashes ('/repositories/...', '/ror.org/...', '/doi.org/...'), the implementation lacks robustness for future cases where relative paths might be passed. The suggested fix using new URL() with a placeholder base normalizes URL parsing and handles edge cases more reliably:
const defaultActiveKeys = [
"authors-facets",
"connection-type-facets",
"organization-relation-type-facets",
"published-facets",
"work-type-facets",
"license-facets",
"language-facets",
"field-of-science-facets",
"registration-agency-facets",
"repository-type-facets",
"client-facets"
]
+
+ const isRepositoriesRoute =
+ new URL(url, 'http://placeholder').pathname.startsWith('/repositories')
return (
<>
@@
- {!url.startsWith('/repositories') && (
+ {!isRepositoriesRoute && (Also applies to line 178.
🤖 Prompt for AI Agents
In `@src/components/WorkFacets/WorkFacets.tsx` around lines 93 - 95, The current
checks using url.startsWith('/repositories') are brittle for relative or
missing-leading-slash inputs; update the logic in WorkFacets (the
component/function where url is inspected) to normalize the URL before checking
the path: create a URL object with a dummy base (e.g. new URL(url,
'http://example')) and use its pathname.startsWith('/repositories') for the
route check, and apply the same normalization for the other occurrence mentioned
(the check at the second location around line 178) so both branches reliably
detect repository routes regardless of leading slash or relative paths.
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
funded-ui
|
| Run status |
|
| Run duration | 01m 42s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
2
|
Pending
|
3
|
Skipped
|
0
|
Passing
|
47
|
| View all changes introduced in this branch ↗︎ | |
| fieldOfScience: searchParams['field-of-science'], | ||
| registrationAgency: searchParams['registration-agency'], | ||
| organizationRelationType: searchParams['organization-relation-type'] || 'allRelated', | ||
| clientId: searchParams['client-id'], |
There was a problem hiding this comment.
Duplicated mapSearchParams logic across four route files
Medium Severity
Four mapSearchParams.ts files (doi.org, orcid.org, ror.org, repositories) contain nearly identical logic for mapping URL search params to query variables. The core mapping of ~10 fields (filterQuery, cursor, contributor, published, resourceTypeId, etc.) is duplicated verbatim. This PR adds organizationRelationType and clientId to three of these files, extending the duplication. Consider extracting a shared utility function that accepts options for route-specific fields like connectionType, clientType, and sort.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/WorkFacets/WorkFacets.tsx (1)
82-93: Consider derivingdefaultActiveKeysfrom rendered facets. This keeps accordion state aligned when repository facets are hidden on repository routes.Suggested refactor
+ const isRepositoriesRoute = /* your repository route check */ const defaultActiveKeys = [ "authors-facets", "connection-type-facets", "organization-relation-type-facets", "published-facets", "work-type-facets", "license-facets", "language-facets", "field-of-science-facets", - "registration-agency-facets", - "repository-type-facets", - "client-facets" + "registration-agency-facets", + ...(isRepositoriesRoute ? [] : ["repository-type-facets", "client-facets"]) ]
| const url = '/doi.org/' + doi + '/' | ||
| return ( | ||
| <Container fluid> | ||
| <Row> | ||
| <Col md={{ offset: 3 }}> | ||
| <h3 className="member-results" id="title">Related Works - {displayedConnectionTitle}</h3> | ||
| <Col md={3} className="d-none d-md-block"> | ||
| <SearchBox path={url} /> | ||
| </Col> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the files mentioned in the review
find . -name "RelatedContent.tsx" -type fRepository: datacite/akita
Length of output: 277
🏁 Script executed:
# Also find SearchBox.tsx
find . -name "SearchBox.tsx" -type fRepository: datacite/akita
Length of output: 98
🏁 Script executed:
# Get the structure to understand the layout
git ls-files | grep -E "(SearchBox|RelatedContent)" | head -20Repository: datacite/akita
Length of output: 597
🏁 Script executed:
# Examine RelatedContent.tsx around lines 56-62
cat -n ./src/app/\(main\)/doi.org/\[...doi\]/RelatedContent.tsx | sed -n '50,70p'Repository: datacite/akita
Length of output: 787
🏁 Script executed:
# Get full context of the file to understand searchParams usage
wc -l ./src/app/\(main\)/doi.org/\[...doi\]/RelatedContent.tsxRepository: datacite/akita
Length of output: 113
🏁 Script executed:
# Read the full RelatedContent.tsx file
cat -n ./src/app/\(main\)/doi.org/\[...doi\]/RelatedContent.tsxRepository: datacite/akita
Length of output: 3590
🏁 Script executed:
# Now examine SearchBox.tsx to see the onSubmit implementation
cat -n ./src/components/SearchBox/SearchBox.tsxRepository: datacite/akita
Length of output: 2430
🏁 Script executed:
# Check if the same pattern exists in other RelatedContent files
for file in ./src/app/\(main\)/*/\[*\]/RelatedContent.tsx; do
echo "=== $file ==="
grep -n "const url = " "$file" 2>/dev/null || echo "No 'const url' found"
doneRepository: datacite/akita
Length of output: 470
🏁 Script executed:
# Check WorksListing to understand how it handles URL params
grep -n "url" ./src/components/WorksListing/WorksListing.tsx | head -20Repository: datacite/akita
Length of output: 240
🏁 Script executed:
# Let's look at how WorksListing uses the url prop
cat ./src/components/WorksListing/WorksListing.tsx | head -100Repository: datacite/akita
Length of output: 3008
🏁 Script executed:
# Check how facets/filters are applied to understand what params should be preserved
grep -n "connectionType\|client-id\|facets" ./src/app/\(main\)/doi.org/\[...doi\]/RelatedContent.tsxRepository: datacite/akita
Length of output: 439
🏁 Script executed:
# Look at how params are mapped
cat -n ./src/app/\(main\)/doi.org/\[...doi\]/mapSearchParams.tsRepository: datacite/akita
Length of output: 1560
🏁 Script executed:
# Check what WorkFacets does with the URL
cat -n ./src/components/WorkFacets/WorkFacets.tsx | head -80Repository: datacite/akita
Length of output: 3644
🏁 Script executed:
# Check if other RelatedContent files also use SearchBox the same way
for file in ./src/app/\(main\)/*/\[*\]/RelatedContent.tsx; do
echo "=== $(basename $(dirname $file))/RelatedContent.tsx ==="
grep -A5 "SearchBox" "$file" 2>/dev/null || echo "No SearchBox found"
doneRepository: datacite/akita
Length of output: 1370
🏁 Script executed:
# Check if there's a clear button or param clearing in SearchBox
grep -n "onSearchClear\|replace" ./src/components/SearchBox/SearchBox.tsxRepository: datacite/akita
Length of output: 158
🏁 Script executed:
# Let's verify the actual behavior by checking if params are preserved anywhere
grep -rn "useSearchParams\|searchParams" ./src/components/SearchBox/ 2>/dev/nullRepository: datacite/akita
Length of output: 240
🏁 Script executed:
# Verify the fixes would work by checking URL construction
node <<'EOF'
// Test the proposed SearchBox fix logic
const testPath = "/doi.org/10.12345/?connection-type=references&client-id=abc";
const searchInput = "water";
const [base, qs] = testPath.split('?');
const params = new URLSearchParams(qs ?? '');
params.set('filterQuery', searchInput);
const result = `${base}?${params.toString()}`;
console.log("Input path:", testPath);
console.log("Search input:", searchInput);
console.log("Result:", result);
console.log("All params preserved:", result.includes('connection-type=references') && result.includes('client-id=abc'));
EOFRepository: datacite/akita
Length of output: 262
🏁 Script executed:
# Verify the RelatedContent fix logic
node <<'EOF'
// Test the proposed RelatedContent fix logic
const searchParamsObj = new URLSearchParams("connection-type=references&client-id=abc&filterQuery=old");
const url = "/doi.org/10.12345/";
const baseQuery = new URLSearchParams(searchParamsObj.toString());
baseQuery.delete('filterQuery');
const searchPath = baseQuery.toString() ? `${url}?${baseQuery.toString()}` : url;
console.log("Input URL:", url);
console.log("Current params:", searchParamsObj.toString());
console.log("Search path (without filterQuery):", searchPath);
console.log("Filters preserved:", searchPath.includes('connection-type=references') && searchPath.includes('client-id=abc'));
console.log("FilterQuery removed:", !searchPath.includes('filterQuery'));
EOFRepository: datacite/akita
Length of output: 305
Preserve query parameters when submitting searches in SearchBox.
At lines 56–61, SearchBox receives a bare base URL (/doi.org/{doi}/) and constructs navigation with only ?filterQuery=..., dropping active filter parameters like connection-type, client-id, facet selections, and pagination. This breaks filter persistence when users search after applying filters.
The issue also affects SearchBox.onSearchClear() at line 36, which uses router.replace(path) and loses the current filterQuery parameter.
Apply both fixes below to preserve and merge query parameters correctly:
✅ Fix 1: RelatedContent.tsx (lines 56–61)
- const url = '/doi.org/' + doi + '/'
+ const url = '/doi.org/' + doi + '/'
+ const baseQuery = new URLSearchParams(searchParams.toString())
+ baseQuery.delete('filterQuery')
+ const searchPath = baseQuery.toString() ? `${url}?${baseQuery.toString()}` : url
return (
<Container fluid>
<Row>
<Col md={3} className="d-none d-md-block">
- <SearchBox path={url} />
+ <SearchBox path={searchPath} />
</Col>✅ Fix 2: SearchBox.tsx (lines 19–36)
const onSubmit = () => {
- if (router)
- router.push(`${path}?filterQuery=${searchInput}`)
+ if (!router) return
+ const [base, qs] = path.split('?')
+ const params = new URLSearchParams(qs ?? '')
+ params.set('filterQuery', searchInput)
+ router.push(`${base}?${params.toString()}`)
}
const onSearchChange = (text: string): void => {
setSearchInput(text)
}
const onSearchClear = () => {
setSearchInput('')
- router.replace(path);
+ const [base, qs] = path.split('?')
+ const params = new URLSearchParams(qs ?? '')
+ params.delete('filterQuery')
+ router.replace(params.toString() ? `${base}?${params.toString()}` : base);
}Apply the same pattern to src/app/(main)/repositories/[...repoid]/RelatedContent.tsx and src/app/(main)/ror.org/[rorid]/RelatedContent.tsx, which use SearchBox identically.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = '/doi.org/' + doi + '/' | |
| return ( | |
| <Container fluid> | |
| <Row> | |
| <Col md={{ offset: 3 }}> | |
| <h3 className="member-results" id="title">Related Works - {displayedConnectionTitle}</h3> | |
| <Col md={3} className="d-none d-md-block"> | |
| <SearchBox path={url} /> | |
| </Col> | |
| const url = '/doi.org/' + doi + '/' | |
| const baseQuery = new URLSearchParams(searchParams.toString()) | |
| baseQuery.delete('filterQuery') | |
| const searchPath = baseQuery.toString() ? `${url}?${baseQuery.toString()}` : url | |
| return ( | |
| <Container fluid> | |
| <Row> | |
| <Col md={3} className="d-none d-md-block"> | |
| <SearchBox path={searchPath} /> | |
| </Col> |
There was a problem hiding this comment.
I'd like to address this in a later PR
There was a problem hiding this comment.
@jrhoads, I understand—thanks for considering it! Would you like me to create a GitHub issue to track this fix for a later PR? I can capture the details about preserving query parameters in SearchBox and the affected RelatedContent files.
| fieldOfScience: searchParams['field-of-science'], | ||
| registrationAgency: searchParams['registration-agency'], | ||
| organizationRelationType: searchParams['organization-relation-type'] || 'allRelated', | ||
| clientId: searchParams['client-id'], |
There was a problem hiding this comment.
Duplicated mapSearchParams logic across route directories
Medium Severity
The mapSearchparams function and SearchParams interface are duplicated across doi.org, orcid.org, and ror.org route directories with nearly identical implementations. All three map the same core fields (filterQuery, cursor, contributor, published, resourceTypeId, language, license, fieldOfScience, registrationAgency, organizationRelationType, clientId) with only minor variations. This PR had to add client-id to all three files separately.
There was a problem hiding this comment.
I'd like to address this in some later work
jrhoads
requested review from
a team,
KellyStathis,
bklaing2 and
codycooperross
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/`(main)/orcid.org/[orcid]/RelatedContent.tsx:
- Line 54: The base URL variable url currently includes a trailing slash (const
url = '/orcid.org/' + orcid + '/'), which leads to a double slash when later
concatenating '/?' (and similar) — update the definition of url to omit the
trailing slash (e.g., '/orcid.org/' + orcid) or change concatenations to use '?'
instead of '/?' so that constructions like url + '?' produce a single slash;
modify usages around the RelatedContent component where url is joined (lines
referencing url + '/?' near the WorksListing calls) to match the new pattern.
🧹 Nitpick comments (1)
cypress/e2e/searchWork.test.ts (1)
23-34: Consider updating the minimum length assertion for consistency.The test checks 9 specific facets (indices 0-8), but the minimum length assertion on line 24 only requires
.at.least(4). While the individual assertions will catch missing facets, updating this to.at.least(9)would provide a clearer failure message if facets are unexpectedly removed.💡 Suggested improvement
.should(($facet) => { - expect($facet).to.have.length.at.least(4) + expect($facet).to.have.length.at.least(9) expect($facet.eq(0)).to.contain('Creators')
| </span> | ||
| </OverlayTrigger> | ||
| ) | ||
| } |
There was a problem hiding this comment.
New InfoTooltip duplicates existing HelpIcon component
Low Severity
The new InfoTooltip component duplicates functionality of the existing HelpIcon component. Both use OverlayTrigger, Tooltip, and faQuestionCircle from the same libraries to render a question mark icon with tooltip text. The stopPropagation and accessibility attributes needed by InfoTooltip could be added as options to HelpIcon instead of creating a separate component.
There was a problem hiding this comment.
Will address in later work
…ss related content pages
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| <span className={styles.facetTitle}> | ||
| {facet.title} | ||
| {tooltipText && <InfoTooltip text={tooltipText} />} | ||
| </span> |
There was a problem hiding this comment.
Tooltip click inside Link causes unintended page navigation
Low Severity
The InfoTooltip is now rendered inside a Next.js Link (<a> tag) in FacetListItem. The tooltip's onClick handler calls e.stopPropagation(), which prevents the Link's click handler from firing. Since the Link's handler is responsible for calling preventDefault() to intercept native navigation, the browser instead follows the <a> href natively, causing a full page reload. Clicking the help icon on organization relation type facets triggers an unintended navigation to the facet URL rather than doing nothing.
Additional Locations (1)
| license: searchParams.license, | ||
| fieldOfScience: searchParams['field-of-science'], | ||
| registrationAgency: searchParams['registration-agency'], | ||
| organizationRelationType: searchParams['organization-relation-type'] || 'allRelated', |
There was a problem hiding this comment.
Unnecessary organizationRelationType default in DOI and ORCID mapSearchParams
Low Severity
The organizationRelationType variable with a default of 'allRelated' is added to the DOI and ORCID mapSearchParams, but these pages never display an Organization Relation Type facet and have no rorId. The value is consumed by buildOrgQuery, which immediately returns an empty string when rorId is absent, making it a no-op. It only affects the react-query cache key, causing unnecessary cache key variation across these pages. Unlike clientId (which correctly enables the new Repository facet), this variable serves no purpose here and was likely copy-pasted from the ror.org mapSearchParams.
Additional Locations (1)
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
1.42.0
|
| Run status |
|
| Run duration | 01m 45s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
1
|
|
Pending
|
3
|
|
Skipped
|
0
|
|
Passing
|
47
|
| View all changes introduced in this branch ↗︎ | |
3 participants
closes: https://github.com/datacite/product-backlog/issues/516
closes: https://github.com/datacite/product-backlog/issues/517
Purpose
This PR introduces filtering improvements to the search experience across DOI, ORCID, ROR, and Repository pages. It specifically adds repository-based filtering and enhances the user interface with better spacing, consistent search box placement, and informative tooltips for connection types.
Approach
The changes reorganize the layout of the search pages to place the search box in the sidebar consistently. It also introduces a new
clientsfacet (Repository) and reusable tooltip components for organization relation types.Key Modifications
SearchBoxto the sidebar acrossdoi.org,repositories, andror.orgpages.client-idparameter support and a corresponding "Repository" facet in the sidebar.ORGANIZATION_RELATION_TYPE_TOOLTIPSto explain filters like "Affiliated Researcher", "Created By", and "Funded By".InfoTooltipcomponent to reduce duplication inFacetList.MetricsDisplayandWorksDashboardstyling for better vertical rhythm and alignment.Important Technical Details
mapSearchParamslogic in multiple routes to extract and passclient-idto the data managers.Facettype and GraphQL queries to includeclientsand optionaltooltipText.Types of changes
Reviewer, please remember our guidelines:
Note
Medium Risk
Touches search query parameter mapping, facet data/types, and shared listing UI across several routes, so regressions could affect filtering and navigation; changes are mostly additive UI/data plumbing with limited logic complexity.
Overview
Adds repository-level filtering across Works/Related Works views by introducing a new
clientsfacet and wiring theclient-idquery param through search/related-contentmapSearchParamsand the DOI search page variables.Refactors the sidebar UX: moves/standardizes the
SearchBoxinto theWorksListingfacets column (via a newsearchBoxslot), adjusts related-content headers/layout, and updates the e2e facet expectations (inserting a new “Repository” facet before “Repository Type”).Improves facet usability by extracting a reusable
InfoTooltip, adding optional per-facettooltipTextsupport (including organization relation type tooltips), and applying minor styling tweaks (metrics spacing, dashboard container classes).Written by Cursor Bugbot for commit 3f8168b. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Improvements