Hero banner: move heading copy to configurable text in hero banner JSON#3335
Merged
Hero banner: move heading copy to configurable text in hero banner JSON#3335
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ENVS.md (1)
151-156:⚠️ Potential issue | 🟡 MinorDoc table now conflicts with its own note and fallback behavior.
Line 151 says all hero banner properties are arrays, but Line 155 introduces
textas a scalar string. Also, the default should mention both runtime fallbacks ({ chain_name } explorerand{ chain_name } blockchain explorerwhen SEO enhanced data is enabled).📝 Suggested doc fix
-_Note_ Here, all values are arrays of up to two strings. The first string represents the value for the light color mode, and the second string represents the value for the dark color mode. If the array contains only one string, it will be used for both color modes. +_Note_ Most values below are arrays of up to two strings for light/dark modes. The `text` property is a plain string. -| text | `string` | The text on the banner | - | `{ chain_name } explorer` | `Duck migration observer` | +| text | `string` | The text on the banner | - | `{ chain_name } explorer` (or `{ chain_name } blockchain explorer` when `NEXT_PUBLIC_SEO_ENHANCED_DATA_ENABLED=true`) | `Duck migration observer` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ENVS.md` around lines 151 - 156, Update the docs to make the hero banner schema consistent with the note and fallback behavior: change the "text" row to indicate it accepts either `string` or `[string]` (or explicitly state "string or array of up to two strings") to match the note about arrays, clarify that all banner properties (e.g., `text`, `background`) can be an array of up to two strings, and update the "Default value" for `text` to show both runtime fallbacks (`{ chain_name } explorer` and `{ chain_name } blockchain explorer` when SEO enhanced data is enabled) so the table and the preceding note are aligned.
🧹 Nitpick comments (1)
ui/home/HeroBanner.pw.tsx (1)
27-27: Extract the hero banner ENV payload into a reusable mock fixture.The long inline JSON at Line 27 hurts readability and reuse in Playwright tests. Please move it to a
mocks/export and import it here.Based on learnings: Applies to **/*.pw.tsx : In Playwright tests, import mock values from existing files in
mocks/rather than hardcoding them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/home/HeroBanner.pw.tsx` at line 27, Replace the long inline JSON ENV payload used in HeroBanner.pw.tsx with a reusable mock export: create or update a mocks file (e.g., export const HERO_BANNER_MOCK = {...}) that contains the JSON for NEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIG and then import that mock into HeroBanner.pw.tsx and use it where the inline array/object is currently provided; update the test to reference the imported HERO_BANNER_MOCK instead of the hardcoded string so other Playwright tests under *.pw.tsx can reuse the same fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/scripts/og_image_generator.js`:
- Line 26: The title assignment uses the logical OR which treats an explicit
empty string as falsy; change the fallback logic in the title expression so it
uses nullish coalescing (referencing bannerConfig.text and the title property) —
replace the `||` fallback with `??` so an explicitly provided empty string is
preserved and the environment-based default `${
process.env.NEXT_PUBLIC_NETWORK_NAME } explorer` is only used when
bannerConfig.text is null or undefined.
In `@deploy/tools/envs-validator/test/.env.multichain`:
- Line 13: The env value for NEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIG is not
quoted and triggers dotenv/lint warnings; wrap the entire JSON-like value in
quotes (use single quotes to follow repo convention) so the line becomes
NEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIG='{"background":[...],"text_color":[...],"text":"..."}'—ensure
you preserve the inner JSON structure and single-quote the whole value for
consistent parsing and linting.
In `@ui/home/HeroBanner.tsx`:
- Around line 52-60: The current IIFE for const text treats an empty string as
absent; replace the IIFE with a nullish-coalescing expression so an explicitly
provided value (including empty string) is preserved: use
config.UI.homepage.heroBanner?.text ?? (config.meta.seo.enhancedDataEnabled ?
`${config.chain.name} blockchain explorer` : `${config.chain.name} explorer`) to
compute text, referencing the existing config.UI.homepage.heroBanner?.text,
config.meta.seo.enhancedDataEnabled and config.chain.name symbols.
---
Outside diff comments:
In `@docs/ENVS.md`:
- Around line 151-156: Update the docs to make the hero banner schema consistent
with the note and fallback behavior: change the "text" row to indicate it
accepts either `string` or `[string]` (or explicitly state "string or array of
up to two strings") to match the note about arrays, clarify that all banner
properties (e.g., `text`, `background`) can be an array of up to two strings,
and update the "Default value" for `text` to show both runtime fallbacks (`{
chain_name } explorer` and `{ chain_name } blockchain explorer` when SEO
enhanced data is enabled) so the table and the preceding note are aligned.
---
Nitpick comments:
In `@ui/home/HeroBanner.pw.tsx`:
- Line 27: Replace the long inline JSON ENV payload used in HeroBanner.pw.tsx
with a reusable mock export: create or update a mocks file (e.g., export const
HERO_BANNER_MOCK = {...}) that contains the JSON for
NEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIG and then import that mock into
HeroBanner.pw.tsx and use it where the inline array/object is currently
provided; update the test to reference the imported HERO_BANNER_MOCK instead of
the hardcoded string so other Playwright tests under *.pw.tsx can reuse the same
fixture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: beba6fd9-22a1-4f63-a2e1-a76e156f8967
⛔ Files ignored due to path filters (2)
ui/home/__screenshots__/HeroBanner.pw.tsx_dark-color-mode_customization-dark-mode-1.pngis excluded by!**/*.pngui/home/__screenshots__/HeroBanner.pw.tsx_default_customization-dark-mode-1.pngis excluded by!**/*.png
📒 Files selected for processing (7)
deploy/scripts/og_image_generator.jsdeploy/tools/envs-validator/schemas/ui.tsdeploy/tools/envs-validator/test/.env.multichaindocs/ENVS.mdtypes/homepage.tsui/home/HeroBanner.pw.tsxui/home/HeroBanner.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description and Related Issue(s)
Resolves #3290
The hero banner heading was always derived from the chain name plus a fixed "explorer" / "blockchain explorer" suffix. This change adds an optional
textfield to the existingNEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIGJSON so deployments can override the full heading string for richer customization, while preserving the previous default whentextis omitted.Proposed Changes
texttoHeroBannerConfigand validate it in the envs validator.HeroBanner.tsx, useconfig.UI.homepage.heroBanner?.textwhen set; otherwise keep the same defaults as before ({chain} explorervs{chain} blockchain explorerwhen SEO enhanced data is enabled).deploy/scripts/og_image_generator.js, usebannerConfig.textfor the OG title when present so Open Graph previews match the customized heading.textfield indocs/ENVS.md(hero banner config table).Environment variable changes
NEXT_PUBLIC_HOMEPAGE_HERO_BANNER_CONFIG: added optional nested fieldtext(string). When set, it replaces the default hero heading (and the OG generator uses it for the title). When omitted, behavior matches the previous hardcoded{network} explorer/{network} blockchain explorerlogic.Breaking or Incompatible Changes
None. Existing configs without
textbehave as before.Additional Information
Issue reference: Hero banner: move 'explorer' to new ENV.
Checklist for PR author