Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Universal AI promotional banner to the search results page, gated behind a PostHog feature flag and displayed only for blank queries or specific AI-related search terms (per HQ issue #10802).
Changes:
- Introduces
UniversalAIBannerwith term-matching logic and a CTA linking to the UAI program page. - Renders the banner within
SearchDisplayand adds a newFeatureFlags.UniversalAISearchBannerenum value. - Adds SearchPage test coverage for banner visibility based on feature flag + query term.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frontends/main/src/page-components/SearchDisplay/UniversalAIBanner.tsx | New banner component with search-term matching + feature flag gating + CTA link |
| frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx | Mounts the banner in the search results UI |
| frontends/main/src/common/feature_flags.ts | Adds universal-ai-search-banner flag identifier |
| frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx | Adds tests verifying banner show/hide behavior |
| const BANNER_SEARCH_TERMS_PATTERN = | ||
| /^(universal ai|ai|ai basics|ai intro|ai introduction|ai fundamentals|ai foundations|artificial intelligence|machine learning|data analytics|deep learning|prescriptive ai|predictive ai|multimodal ai|large language models|large language model|llms|llm|generative ai|gen ai|ai ethics|optimization|python|computer vision|ai framework|ai models|ai agents|agentic ai|ai and sustainability|ai and health|ai and healthcare|ai and medicine|ai and entrepreneurship)$/i | ||
|
|
There was a problem hiding this comment.
BANNER_SEARCH_TERMS_PATTERN is a very large, hard-to-maintain regex for an exact-match term list. Since you're effectively doing case-insensitive membership in a fixed list, consider storing the allowed terms in a normalized array/Set (e.g., term.trim().toLowerCase()) and checking set.has(...) instead of a single monolithic regex; it will be easier to update and review safely.
| const BannerLink = styled.a(({ theme }) => ({ | ||
| color: theme.custom.colors.silverGrayDark, | ||
| fontWeight: theme.typography.fontWeightMedium, | ||
| fontSize: "12px", | ||
| whiteSpace: "nowrap", |
There was a problem hiding this comment.
BannerLink is an internal navigation link (to a /programs/... route) but it uses a plain <a> element. In this codebase, internal links generally use next/link to avoid full page reloads and enable prefetching; consider switching BannerLink to be styled(Link) (or otherwise wrap with Next.js Link).
| required. | ||
| </BannerDescription> | ||
| </BannerContent> | ||
| <BannerLink href={ctaHref}>Learn More</BannerLink> |
There was a problem hiding this comment.
The CTA text "Learn More" is generic, which makes link lists ambiguous for screen-reader users. Consider making the accessible name specific (e.g., "Learn more about Universal AI"), either by changing the visible text or adding an aria-label that includes the program name.
| <BannerLink href={ctaHref}>Learn More</BannerLink> | |
| <BannerLink | |
| href={ctaHref} | |
| aria-label="Learn more about Universal AI" | |
| > | |
| Learn More | |
| </BannerLink> |
| "&:hover": { | ||
| backgroundColor: theme.custom.colors.lightGray1, | ||
| fontWeight: theme.typography.fontWeightBold, | ||
| }, |
There was a problem hiding this comment.
BannerLink adds hover styling but no visible keyboard focus styling. For accessibility, add an &:focus-visible style (outline/background/etc.) consistent with other interactive elements in this area so keyboard users can see focus.
| }, | |
| }, | |
| "&:focus-visible": { | |
| backgroundColor: theme.custom.colors.lightGray1, | |
| fontWeight: theme.typography.fontWeightBold, | |
| outline: `2px solid ${theme.custom.colors.red}`, | |
| outlineOffset: "2px", | |
| }, |
There was a problem hiding this comment.
switching from <a> to <Link> fixes this automatically
shanbady
left a comment
There was a problem hiding this comment.
works as expected so approving 👍
I had two thoughts/questions:
- maybe on the next iteration/if we will be tweaking this - is it possible to move the keyword array and the copy+link text into posthog as a payload?
- double check if this makes sense but i'm wondering if we are trying to match on any mention of the word "ai" or it is this specific list (otherwise i think we can condernse this list tremendously)
OpenAPI ChangesNo detectable change. Unexpected changes? Ensure your branch is up-to-date with |
eb281c5 to
f351342
Compare
What are the relevant tickets?
closes https://github.com/mitodl/hq/issues/10802
Description (What does it do?)
The pr adds a banner that links to the UAI program on the search page. The banner should be shown when
There is a feature flag for the banner
Screenshots (if appropriate):
How can this be tested?
Go to search and ensure it works normally
Create a feature flag called "universal-ai-search-banner" in posthog and turn the feature on
When there is no search term you should see the banner
When the search term is one of the terms in the ticket you should see the banner
When there is any other search term you should not see a banner