Skip to content

SCIX-850 feat(citation): redesign citation modal, gate settings link by auth - #904

Open
thostetler wants to merge 3 commits into
adsabs:masterfrom
thostetler:fix/scix-850-copy-citation-keyformat
Open

SCIX-850 feat(citation): redesign citation modal, gate settings link by auth#904
thostetler wants to merge 3 commits into
adsabs:masterfrom
thostetler:fix/scix-850-copy-citation-keyformat

Conversation

@thostetler

@thostetler thostetler commented Jul 22, 2026

Copy link
Copy Markdown
Member

Copy Citation (abstract-page share options and result-list share menu)
ignored the user's custom BibTeX export key format from account
settings, unlike the full export page. The modal itself was also due
for a visual refresh and a way to reach the citation format setting.

  1. Add getBibtexExportParams helper deriving keyformat/journalformat/authorcutoff/maxauthor from settings, bibtex vs bibtexabs aware
  2. Wire it into AbstractCitationModal and ItemResourceDropdowns
  3. Sanitize keyformat via purifyString to match the export page
  4. Redesign the Copy Citation modal to match the new mockup
  5. Footer links to advanced export options and citation format settings, disabled with a tooltip for anonymous users
image

@thostetler
thostetler force-pushed the fix/scix-850-copy-citation-keyformat branch from 3101b07 to a31002c Compare July 24, 2026 16:38
@thostetler thostetler changed the title SCIX-850 fix(export): pass bibtex keyformat settings to Copy Citation SCIX-850 feat(citation): redesign citation modal, gate settings link by auth Jul 24, 2026
@thostetler
thostetler requested a review from shinyichen July 24, 2026 16:40
@thostetler
thostetler marked this pull request as ready for review July 24, 2026 16:44
Copilot AI review requested due to automatic review settings July 24, 2026 16:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Risk summary: Low. Changes are localized to citation export parameter derivation and the Copy Citation modal UI, with added unit tests covering the new behavior.

Changes:

  • Added getBibtexExportParams helper to consistently derive BibTeX export params (incl. keyformat sanitization) from user settings.
  • Wired BibTeX-specific params into Copy Citation flows (abstract citation modal + result-list share menu) so they respect account settings.
  • Redesigned AbstractCitationModal footer actions and gated the settings link behind authentication (tooltip + disabled control for anonymous users).

Findings (priority order):

  • low (naming): One new test description mentions “logged-in user” but does not set auth state and does not assert auth-dependent behavior. (See stored PR comment.)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/components/ResultList/Item/ItemResourceDropdowns.tsx Passes BibTeX-only export params into the share-menu export citation query.
src/components/ResultList/Item/ItemResourceDropdowns.test.tsx Adds coverage ensuring BibTeX params are included only for BibTeX formats.
src/components/AbstractDetails/AbstractCitationModal.tsx Redesigns the citation modal UI and applies BibTeX params + auth-gated settings link.
src/components/AbstractDetails/AbstractCitationModal.test.tsx Adds coverage for BibTeX param inclusion and authenticated vs anonymous footer behavior.
src/api/export/getBibtexExportParams.ts Introduces helper to derive/export BibTeX-only query params from settings with keyformat sanitization.
src/api/export/getBibtexExportParams.test.ts Adds unit tests for helper behavior across BibTeX / BibTeX ABS / non-BibTeX formats and sanitization.

});
});

test('includes custom bibtex params for a logged-in user when the default format is bibtex', async () => {
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.90110% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.8%. Comparing base (0e0d168) to head (959530b).

Files with missing lines Patch % Lines
...mponents/AbstractDetails/AbstractCitationModal.tsx 98.5% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #904     +/-   ##
========================================
+ Coverage    67.8%   67.8%   +0.1%     
========================================
  Files         354     358      +4     
  Lines       41672   41975    +303     
  Branches     2187    2204     +17     
========================================
+ Hits        28223   28437    +214     
- Misses      13400   13489     +89     
  Partials       49      49             
Files with missing lines Coverage Δ
src/api/export/getBibtexExportParams.ts 100.0% <100.0%> (ø)
...mponents/ResultList/Item/ItemResourceDropdowns.tsx 77.8% <100.0%> (+0.2%) ⬆️
...mponents/AbstractDetails/AbstractCitationModal.tsx 91.4% <98.5%> (ø)

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy Citation (abstract-page share options and result-list share menu)
ignored the user's custom BibTeX export key format from account
settings, unlike the full export page.

- Add getBibtexExportParams helper deriving keyformat/journalformat/
  authorcutoff/maxauthor from settings, bibtex vs bibtexabs aware
- Wire it into AbstractCitationModal and ItemResourceDropdowns
- Sanitize keyformat via purifyString to match the export page
- Add coverage for custom keyformat, unset settings, and non-bibtex
  formats
…by auth

Redesigns the Copy Citation modal to match the new mockup and makes
the "Copy citation settings" shortcut unusable for logged-out users
instead of silently sending them to a page middleware would block
anyway.

- Bigger modal, visible Format label, prominent blue Copy button
- Footer links to advanced export options and citation format settings
- Anonymous users see a disabled settings button with a tooltip
  explaining why, instead of a live link
- Disabled state uses a real disabled Button behind Tooltip
  shouldWrapChildren so it's keyboard-focusable and screen-reader
  visible, not just a styled div
- Tests cover hover and keyboard-focus tooltip triggers, plus the
  enabled state for logged-in users
Copilot review flagged a test title mentioning a logged-in user
without asserting auth-dependent behavior.
@thostetler
thostetler force-pushed the fix/scix-850-copy-citation-keyformat branch from a31002c to 959530b Compare July 30, 2026 17:49

@shinyichen shinyichen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Just one small question.

</SimpleLink>
{isAuthenticated ? (
<SimpleLink
href="/user/settings/export?tab=3"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does tab=3 do anything? I was taken to the first tab.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm I added it... let me see what's broken now 😅

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.

3 participants