Skip to content

Conversation

bp-cos
Copy link
Contributor

@bp-cos bp-cos commented Jul 14, 2025

  • Ticket: [eng-7264]
  • Feature flag: n/a

Purpose

Added a "caveat" banner to all OSF preprints.

There will be 2 banners displayed on some preprints depending on the use case.

Summary of Changes

Added a conditional statement to the preprint details page to display a banner for all osf preprints
Added logic to the preprint-banner-display component to find the class and message and display the osf banner
Added a feature switch to the environment and router to prevent errors on displaying preprints locally

Screenshot(s)

Screenshot 2025-07-14 at 2 49 52 PM

Side Effects

Some OSF users will see 2 banners.

QA Notes

Enjoy

@bp-cos bp-cos requested a review from futa-ikeda July 14, 2025 19:51
Copy link
Contributor

@futa-ikeda futa-ikeda left a comment

Choose a reason for hiding this comment

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

Looks good to me! There's a couple very minor comments that I had, but nothing that blocks merging

<Preprints::-Components::PreprintStatusBanner
@submission={{this.model.preprint}}
@provider={{this.model.provider}}
@isOSFBanner=true
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: I think this should be {{true}}, though it seems like it's working fine as is?


declare const config: {
WATER_BUTLER_ENABLED: boolean;
PREPRINT_ADAPTER_OPTIONS_ENABLED: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate this idea, as it always takes me a few minutes to realize why my preprint detail page is broken 😄

}

get showOSFBanner(): boolean {
return this.model.provider.id === 'osf';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't think this is needed, as there may be some other criteria in the future to show this banner, but there is also the config.defaultProvider that may be appropriate to use here. We use it in other places to basically check the same thing, but it feels marginally more robust to be using a config rather than a hardcoded providerID

@bp-cos bp-cos merged commit a4387ca into CenterForOpenScience:feature/pbs-25-12 Jul 15, 2025
9 checks passed
@bp-cos bp-cos deleted the bug-fix/eng-7264 branch July 15, 2025 17:45
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.

2 participants