Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds light/dark theme support and wiring: new ThemeContext + ThemeProvider (SSR-aware, persists to localStorage, respects system preference), App wrapped with ThemeProvider, theme toggles added in Layout, CSS variables and Tailwind token updates, and widespread presentational updates to use theme tokens; several data hooks refactored to use params refs. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Layout as Layout (toggle)
participant ThemeProv as ThemeProvider
participant Storage as LocalStorage
participant DOM as documentElement
participant Components as App Components
User->>Layout: click theme toggle
Layout->>ThemeProv: toggleTheme()
ThemeProv->>ThemeProv: update theme state
ThemeProv->>Storage: write "atlas-theme"
ThemeProv->>DOM: set documentElement.dataset.theme
ThemeProv->>Components: provide updated context
Components->>Components: re-render using CSS variables
Components-->>User: UI updates to selected theme
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Summary of ChangesHello @pthmas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new light mode to the frontend, enhancing user experience with customizable and persistent theme options. Concurrently, it delivers a major performance upgrade to the backend indexer by transitioning to an efficient bulk data insertion strategy using PostgreSQL's binary COPY, which drastically reduces database write times and optimizes resource utilization. The changes also include a more modular structure for the indexer's data processing and RPC communication. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a light mode to the frontend, utilizing system preferences and local storage for persistence. It also includes a significant, undocumented backend refactor for the indexer, transitioning from individual sqlx queries to a batch-oriented approach using PostgreSQL's binary COPY and UNNEST for high-performance data ingestion. While these performance improvements are highly beneficial, the lack of documentation for such a major architectural change in the PR description is noted. Technical feedback focuses on optimizing data conversions in the indexer's hot paths and refining the CSS theme implementation to avoid aggressive global overrides that could impact maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
frontend/src/context/ThemeContext.tsx (1)
30-33: Minor FOUC risk: theme is applied to the DOM in auseEffect.
document.documentElement.dataset.themeis set in an effect (post-paint). On the very first render, if the defaultdata-themeon<html>doesn't match the resolved initial theme, there may be a brief flash of the wrong theme. For a client-rendered SPA this is typically acceptable, but if you notice flickering you could setdata-themesynchronously inindex.htmlvia a small inline<script>that readslocalStoragebefore React hydrates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/context/ThemeContext.tsx` around lines 30 - 33, The theme is currently applied in ThemeContext via useEffect (document.documentElement.dataset.theme and window.localStorage.setItem(STORAGE_KEY, theme)), which can cause a brief FOUC on first paint; add a small synchronous inline script in your HTML entry (before the React bundle) that reads localStorage[STORAGE_KEY] (using the same STORAGE_KEY constant) and sets document.documentElement.dataset.theme to that value (or to the ThemeContext default if missing) so the correct data-theme is present before hydration; ensure the inline script runs before the app script to avoid flicker.frontend/src/components/ProxyBadge.tsx (1)
46-48:text-[0.65rem]is redundant on both badge usages.
.badge-chipalready setsfont-size: 0.65rem; the Tailwind override adds no effect on either theProxyBadgespan or theProxyIndicatorspan.♻️ Suggested cleanup
- <span className="badge-chip text-[0.65rem]"> + <span className="badge-chip"> Proxy </span><span - className="badge-chip text-[0.65rem]" + className="badge-chip" title={`${getProxyTypeLabel(proxyInfo.proxy_type)} Proxy - ...`} >Also applies to: 80-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ProxyBadge.tsx` around lines 46 - 48, The spans in ProxyBadge and ProxyIndicator duplicate font-size by including the Tailwind class text-[0.65rem] even though .badge-chip already sets font-size:0.65rem; remove the redundant text-[0.65rem] from the className strings on the span inside the ProxyBadge component and the span inside the ProxyIndicator (the other badge usage referenced) so they rely solely on "badge-chip" for font sizing.frontend/src/pages/TokensPage.tsx (1)
121-123:text-[0.65rem]is redundant alongsidebadge-chip.
.badge-chipalready declaresfont-size: 0.65reminindex.css, so the Tailwind utility class on this element is a no-op.♻️ Suggested cleanup
- <span className="badge-chip uppercase tracking-wide text-[0.65rem]"> + <span className="badge-chip uppercase tracking-wide">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/TokensPage.tsx` around lines 121 - 123, The span rendering token.symbol uses both the custom CSS class badge-chip and the Tailwind utility text-[0.65rem]; remove the redundant Tailwind class from the className on that span (leave badge-chip and other utilities like uppercase/tracking-wide intact) so font-size is controlled only by the existing .badge-chip rule; search for similar occurrences and remove duplicate font-size utilities where badge-chip is used.frontend/src/index.css (1)
154-172: Consider semantic CSS variables over!importantoverrides for text colors.Overriding Tailwind utility classes with
!importantworks but makes the theming brittle — any new consumer oftext-gray-*is silently affected, and the overrides are invisible at the call site.A more scalable approach would be to expose semantic CSS variables (e.g.,
--color-text-primary,--color-text-secondary) toggled per theme, and reference them in the Tailwind config. This avoids the!importantchain entirely and makes theme-awareness explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 154 - 172, The current light-theme CSS uses hard overrides with !important on selectors like [data-theme='light'] .text-white and .text-gray-300/.text-gray-400/.text-gray-500/.text-gray-600 which is brittle; replace these overrides by defining semantic CSS variables on the theme container (e.g., set --color-text-primary, --color-text-secondary, --color-text-muted, etc. inside [data-theme='light']) and remove the !important rules, then update Tailwind configuration (or the utility mapping) to reference those variables for the corresponding text color utilities (so utilities like text-white and text-gray-* resolve to var(--color-text-...)). Ensure variable names are clear and used consistently across styles instead of direct color overrides.backend/crates/atlas-indexer/src/indexer.rs (1)
81-87:copy_clientis created once with no reconnection — confirm this is handled by the outer retry.If the PostgreSQL connection drops mid-run, all
write_batchcalls will fail. This appears to be handled because the error propagates up torun(), which returnsErr, andrun_with_retryinmain.rsrestartsrun()(creating a newcopy_client). Worth a brief comment to make this explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-indexer/src/indexer.rs` around lines 81 - 87, The copy_client used in indexer::run is created once via Self::connect_copy_client and not reconnected on failure; clarify by adding a concise comment near the copy_client creation (referencing run, connect_copy_client, and copy_client) stating that connection failures during write_batch are expected to propagate and that run is restarted by main::run_with_retry which creates a fresh copy_client, so no internal reconnection loop is required; ensure the comment also notes that any early return/Err from write_batch should bubble up to trigger run_with_retry.backend/crates/atlas-indexer/src/copy.rs (1)
11-11: Functions arepubbut only used within the crate — considerpub(crate)for consistency.All callers are in
indexer.rs(same crate). The sibling modules (batch,fetcher) consistently usepub(crate). This applies to all five functions in this file.Example fix for copy_blocks (apply similarly to the other four)
-pub async fn copy_blocks(tx: &mut Transaction<'_>, batch: &BlockBatch) -> Result<()> { +pub(crate) async fn copy_blocks(tx: &mut Transaction<'_>, batch: &BlockBatch) -> Result<()> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-indexer/src/copy.rs` at line 11, Change the exported visibility of functions in this module from public to crate-only: update copy_blocks (and the other four functions in this file) from pub to pub(crate) so they match sibling modules like batch and fetcher and restrict access to within the crate; locate each function declaration (e.g., copy_blocks) and replace the visibility modifier accordingly, keeping signatures and async/return types unchanged.backend/crates/atlas-indexer/src/batch.rs (1)
24-112: The columnar (SoA) layout is efficient for UNNEST but fragile — consider a debug assertion.Adding a debug-mode consistency check would catch misaligned pushes during development without any runtime cost in release builds. For example, a
debug_assert_consistent_lengthsmethod onBlockBatchthat verifies allb_*vecs have the same length, allt_*vecs match, etc. This is optional but would guard against subtle bugs when adding new fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-indexer/src/batch.rs` around lines 24 - 112, The BlockBatch struct uses many parallel Vecs (SoA) and is fragile to misaligned pushes; add a debug-only consistency checker method (e.g., impl BlockBatch::debug_assert_consistent_lengths) that iterates the related groups (block b_* vectors, transaction t_* vectors, event_logs el_* vectors, nft_transfers nt_* vectors, erc20_transfers et_* vectors, and the various lookup vectors like tl_*) and asserts all lengths match their group's canonical length using debug_assert!, and call this helper at strategic mutation points (after collection and before bulk insert) to catch mismatches in dev builds without affecting release performance.backend/crates/atlas-indexer/src/fetcher.rs (1)
156-164: Avoid cloning large JSON values during deserialization.
response_mapstores references, forcingresult.clone()on lines 180 and 199 for every block and receipt set. For blocks with many transactions this copies significant data. Consider storing owned values (e.g.,BTreeMap<u64, serde_json::Value>) and usingresponse_map.remove(&block_id)to move the value intoserde_json::from_valuewithout cloning.Sketch
- let mut response_map: BTreeMap<u64, &serde_json::Value> = BTreeMap::new(); - - for resp in &batch_response { - if let Some(id) = resp.get("id").and_then(|v| v.as_u64()) { - response_map.insert(id, resp); - } - } + let mut response_map: BTreeMap<u64, serde_json::Value> = BTreeMap::new(); + + for resp in batch_response { + if let Some(id) = resp.get("id").and_then(|v| v.as_u64()) { + response_map.insert(id, resp); + } + }Then use
response_map.remove(&block_id)andresponse_map.remove(&receipts_id)downstream to avoid cloning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-indexer/src/fetcher.rs` around lines 156 - 164, Change response_map to own JSON values so you can move them instead of cloning: declare response_map as BTreeMap<u64, serde_json::Value> and when populating it from batch_response insert the owned value (e.g., response_map.insert(id, resp.clone()) or iterate ownership to avoid cloning there). Then replace uses of result.clone() (and similar clones for receipts) with response_map.remove(&block_id) and response_map.remove(&receipts_id) to take ownership and pass the Value directly into serde_json::from_value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/atlas-indexer/src/fetcher.rs`:
- Around line 38-50: The fetch_blocks_batch function can hang without an HTTP
timeout and will underflow/log wrong ranges when count == 0; when constructing
the reqwest::Client (call site in indexer.rs) build it with a sensible timeout
(e.g., Client::builder().timeout(Duration::from_secs(30)).build()?) so RPC calls
fail fast, and in fetch_blocks_batch guard against count == 0 by returning an
empty Vec early before the tracing::debug and the rate_limiter loop (also avoid
executing the for _ in 0..(count * 2) when count is zero) to prevent the
underflow in start_block + count as u64 - 1 and unnecessary waits.
In `@backend/crates/atlas-indexer/src/indexer.rs`:
- Around line 49-79: The current ssl detection in connect_copy_client uses
brittle substring checks on database_url; parse the connection string instead
(e.g., with url::Url or tokio_postgres::Config::from_str) to reliably extract
and decode the "sslmode" query parameter, normalize it to lowercase, and set
needs_tls true for modes that should attempt TLS (at least "require",
"verify-ca", "verify-full", and "prefer"); update the needs_tls logic and remove
the .contains(...) checks so connect_copy_client, needs_tls and database_url
handling work with URL-encoded values and variants.
- Around line 407-414: The fallback on
receipt_map.get(&tx_hash_str).unwrap_or(...) silently treats missing receipts as
failed (status=false, gas_used=0); update the code around
receipt_map/get(&tx_hash_str) in indexer.rs to detect the None case and emit a
warning-level log that includes the tx_hash_str and any relevant context (e.g.,
block or batch identifiers) before falling back, then proceed to set status,
gas_used, contract_created as before; use the project’s existing logging
facility (e.g., tracing::warn! or the module logger) so missing receipts are
visible in logs for debugging.
- Around line 296-303: Retrying mini-batches can regress
indexer_state.last_indexed_block because write_batch currently overwrites the
checkpoint with the retried block's height (e.g., 5), causing reprocessing of
later blocks; fix by changing write_batch to either (A) accept a flag like
skip_checkpoint_for_retry (or add a new write_batch_retry) and skip updating
indexer_state when true so retried mini-batches don't touch last_indexed_block,
or (B) modify the upsert SQL inside write_batch to use
GREATEST(current_last_indexed_block, incoming_last_indexed_block) (or equivalent
DB-specific expression) so the checkpoint never moves backward; update the call
site in the BlockBatch retry path (where collect_block and write_batch are used)
to pass the retry flag if you choose (A) and add a brief test ensuring
last_indexed_block never decreases after a retry.
In `@frontend/src/components/Layout.tsx`:
- Around line 117-151: The theme toggle button (uses toggleTheme and isDark,
currently inside the element with class "hidden md:flex") is not rendered on
small screens; add the same button markup (preserving type="button", aria-label
logic and aria-hidden on icons) into the mobile nav container used for small
screens (the mobile nav section that currently lacks a toggle) or remove the
"hidden md:flex" wrapper so the toggle is visible on all breakpoints; ensure you
keep the same conditional SVG rendering and classes so behavior/accessibility
remains identical.
In `@frontend/src/index.css`:
- Around line 27-29: The body CSS block uses a standalone `@apply` which Biome
flags and Stylelint expects a blank line after; either move the `@apply` usage
into the existing `@layer` components block (where `@apply` is allowed) by placing
the font/antialiased/transition utilities inside the body rule within `@layer`
components, or enable Biome's Tailwind CSS support in the tool config;
additionally, ensure there is a blank line between the `@apply` directive and the
following declarations (background-color/color) to satisfy Stylelint's
declaration-empty-line-before rule.
- Around line 154-168: The light-mode text color scale is inverted and has a
duplicate: ensure the selectors .text-gray-300, .text-gray-400, .text-gray-500,
and .text-gray-600 form a descending visibility gradient (darkest → lightest)
and remove duplicates; specifically update the mappings so .text-gray-300 uses
the darkest value (swap with the current .text-gray-400), .text-gray-400 uses
the next lighter value (the current .text-gray-300 value), .text-gray-500 is a
mid-gray between those two, and .text-gray-600 is the lightest gray—adjust the
color codes for .text-gray-300/.text-gray-400/.text-gray-500/.text-gray-600
selectors accordingly to restore a proper graduated dark-to-light scale.
---
Nitpick comments:
In `@backend/crates/atlas-indexer/src/batch.rs`:
- Around line 24-112: The BlockBatch struct uses many parallel Vecs (SoA) and is
fragile to misaligned pushes; add a debug-only consistency checker method (e.g.,
impl BlockBatch::debug_assert_consistent_lengths) that iterates the related
groups (block b_* vectors, transaction t_* vectors, event_logs el_* vectors,
nft_transfers nt_* vectors, erc20_transfers et_* vectors, and the various lookup
vectors like tl_*) and asserts all lengths match their group's canonical length
using debug_assert!, and call this helper at strategic mutation points (after
collection and before bulk insert) to catch mismatches in dev builds without
affecting release performance.
In `@backend/crates/atlas-indexer/src/copy.rs`:
- Line 11: Change the exported visibility of functions in this module from
public to crate-only: update copy_blocks (and the other four functions in this
file) from pub to pub(crate) so they match sibling modules like batch and
fetcher and restrict access to within the crate; locate each function
declaration (e.g., copy_blocks) and replace the visibility modifier accordingly,
keeping signatures and async/return types unchanged.
In `@backend/crates/atlas-indexer/src/fetcher.rs`:
- Around line 156-164: Change response_map to own JSON values so you can move
them instead of cloning: declare response_map as BTreeMap<u64,
serde_json::Value> and when populating it from batch_response insert the owned
value (e.g., response_map.insert(id, resp.clone()) or iterate ownership to avoid
cloning there). Then replace uses of result.clone() (and similar clones for
receipts) with response_map.remove(&block_id) and
response_map.remove(&receipts_id) to take ownership and pass the Value directly
into serde_json::from_value.
In `@backend/crates/atlas-indexer/src/indexer.rs`:
- Around line 81-87: The copy_client used in indexer::run is created once via
Self::connect_copy_client and not reconnected on failure; clarify by adding a
concise comment near the copy_client creation (referencing run,
connect_copy_client, and copy_client) stating that connection failures during
write_batch are expected to propagate and that run is restarted by
main::run_with_retry which creates a fresh copy_client, so no internal
reconnection loop is required; ensure the comment also notes that any early
return/Err from write_batch should bubble up to trigger run_with_retry.
In `@frontend/src/components/ProxyBadge.tsx`:
- Around line 46-48: The spans in ProxyBadge and ProxyIndicator duplicate
font-size by including the Tailwind class text-[0.65rem] even though .badge-chip
already sets font-size:0.65rem; remove the redundant text-[0.65rem] from the
className strings on the span inside the ProxyBadge component and the span
inside the ProxyIndicator (the other badge usage referenced) so they rely solely
on "badge-chip" for font sizing.
In `@frontend/src/context/ThemeContext.tsx`:
- Around line 30-33: The theme is currently applied in ThemeContext via
useEffect (document.documentElement.dataset.theme and
window.localStorage.setItem(STORAGE_KEY, theme)), which can cause a brief FOUC
on first paint; add a small synchronous inline script in your HTML entry (before
the React bundle) that reads localStorage[STORAGE_KEY] (using the same
STORAGE_KEY constant) and sets document.documentElement.dataset.theme to that
value (or to the ThemeContext default if missing) so the correct data-theme is
present before hydration; ensure the inline script runs before the app script to
avoid flicker.
In `@frontend/src/index.css`:
- Around line 154-172: The current light-theme CSS uses hard overrides with
!important on selectors like [data-theme='light'] .text-white and
.text-gray-300/.text-gray-400/.text-gray-500/.text-gray-600 which is brittle;
replace these overrides by defining semantic CSS variables on the theme
container (e.g., set --color-text-primary, --color-text-secondary,
--color-text-muted, etc. inside [data-theme='light']) and remove the !important
rules, then update Tailwind configuration (or the utility mapping) to reference
those variables for the corresponding text color utilities (so utilities like
text-white and text-gray-* resolve to var(--color-text-...)). Ensure variable
names are clear and used consistently across styles instead of direct color
overrides.
In `@frontend/src/pages/TokensPage.tsx`:
- Around line 121-123: The span rendering token.symbol uses both the custom CSS
class badge-chip and the Tailwind utility text-[0.65rem]; remove the redundant
Tailwind class from the className on that span (leave badge-chip and other
utilities like uppercase/tracking-wide intact) so font-size is controlled only
by the existing .badge-chip rule; search for similar occurrences and remove
duplicate font-size utilities where badge-chip is used.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
backend/crates/atlas-indexer/Cargo.tomlbackend/crates/atlas-indexer/src/batch.rsbackend/crates/atlas-indexer/src/copy.rsbackend/crates/atlas-indexer/src/fetcher.rsbackend/crates/atlas-indexer/src/indexer.rsbackend/crates/atlas-indexer/src/main.rsbackend/migrations/20240107000001_nft_transfers_unique.sqlfrontend/src/App.tsxfrontend/src/components/ContractTypeBadge.tsxfrontend/src/components/Layout.tsxfrontend/src/components/ProxyBadge.tsxfrontend/src/components/StatusBadge.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/index.cssfrontend/src/pages/AddressPage.tsxfrontend/src/pages/TokensPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/tailwind.config.js
pthmas
force-pushed
the
pierrick/light-mode
branch
from
3a80b3b to
ce7e1d8
Compare
tac0turtle
approved these changes
Feb 24, 2026
pthmas
force-pushed
the
pierrick/light-mode
branch
from
eaed4bf to
82440c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/pages/BlocksPage.tsx (1)
26-26:⚠️ Potential issue | 🟠 MajorFix CI failure: unused
tickstateThe
tickvalue is never read, triggeringno-unused-varsand failing CI. You can keep the re-render side effect while discarding the value in the tuple.🔧 Proposed fix
- const [tick, setTick] = useState(0); + const [, setTick] = useState(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/BlocksPage.tsx` at line 26, The declared state variable `tick` in BlocksPage.tsx is unused causing a lint failure; update the useState destructuring to discard the value (e.g., replace `const [tick, setTick] = useState(0)` with a form that omits the first element) so you keep `setTick` for the re-render side effect but remove the unused `tick` identifier.frontend/src/components/Layout.tsx (1)
84-84:⚠️ Potential issue | 🟡 MinorPre-existing CI warning: suppress or fix the missing
displayedHeightdependency.The pipeline flags a
react-hooks/exhaustive-depswarning here. The intent to omitdisplayedHeight(to break a state-update loop) is clear, but the lint warning is unresolved and will accumulate CI noise. The cleanest fix is to replace thedisplayedHeightstate read withdisplayedRef.current(which already mirrors the same value) and remove the state from the condition entirely.💡 Proposed fix
- if (displayedHeight == null || height > displayedRef.current) { + if (displayedRef.current === 0 || height > displayedRef.current) {Alternatively, add an inline suppression comment if the intentional omission must be preserved:
// eslint-disable-next-line react-hooks/exhaustive-deps🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Layout.tsx` at line 84, The useEffect dependency list omits displayedHeight causing an eslint react-hooks/exhaustive-deps warning; fix by reading displayedRef.current instead of the displayedHeight state inside the effect (so the effect no longer depends on displayedHeight) and then remove displayedHeight from the dependency array (leave [height, bps]); update references inside the effect to use displayedRef.current where displayedHeight was used; alternatively, if you must keep the state read, add a single-line suppression comment above the effect (// eslint-disable-next-line react-hooks/exhaustive-deps) — target the effect in Layout component where displayedHeight and displayedRef are used.frontend/src/pages/NFTTokenPage.tsx (1)
109-165:⚠️ Potential issue | 🟠 MajorIncomplete semantic color migration:
<dd>values should use theme-aware semantic classes like<dt>elements.The
<dt>labels (Lines 112, 119, 132, 145, 152) use the semantictext-fg-subtleclass, but their corresponding<dd>values still use the non-semantictext-gray-200class. Whiletext-gray-200is technically theme-aware in this codebase (using CSS variables that adapt to light/dark mode), it breaks the semantic color pattern established for labels.The inconsistency is visible across Lines 114 (
hash text-gray-200), 127 and 140 (---fallbacks withtext-gray-200), and most critically Line 146 where the "Last Transfer" block number is the raw value inside<dd className="text-gray-200">.For consistency with the theming design, migrate these
<dd>text colors to semantic classes liketext-fgortext-fg-subtleto maintain a unified color system throughout the Details panel.Suggested fix
- <span className="hash text-gray-200">{token?.token_id || tokenId || '---'}</span> + <span className="hash text-fg">{token?.token_id || tokenId || '---'}</span> - <span className="text-gray-200">---</span> + <span className="text-fg">---</span> - <dd className="text-gray-200"> + <dd className="text-fg"> {token?.last_transfer_block ? `Block ${formatNumber(token.last_transfer_block)}` : '---'} </dd>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/NFTTokenPage.tsx` around lines 109 - 165, The Details panel uses non-semantic text-gray-200 on several <dd> elements; replace those with the theme-aware semantic class (e.g. text-fg) so values match the label styling: update the span with class "hash text-gray-200", the fallback <span className="text-gray-200">---</span> instances, and the <dd className="text-gray-200"> for Last Transfer to use text-fg (or text-fg-subtle if you want lower emphasis) so all token-related outputs (token?.token_id/tokenId, contract/contractAddress fallbacks, owner fallback, and token.last_transfer_block display) follow the semantic color system.
♻️ Duplicate comments (1)
frontend/src/index.css (1)
57-59:⚠️ Potential issue | 🟠 MajorBiome still rejects Tailwind
@apply(lint failure).Static analysis is flagging
@applyas unsupported. Enable Tailwind parsing in Biome or exclude CSS from Biome linting so CI can pass.#!/bin/bash # Inspect Biome config for Tailwind/CSS settings. fd -a '^biome\.jsonc?$|^biome\.toml$' for f in $(fd -a '^biome\.jsonc?$|^biome\.toml$'); do echo "== $f =="; rg -n "tailwind|css|parser|files|ignore" "$f"; doneAlso applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 57 - 59, Biome is flagging the Tailwind `@apply` usage in the CSS rule "body { `@apply` font-sans antialiased transition-colors duration-200; }"; update the Biome configuration to either enable Tailwind/PostCSS parsing (so `@apply` is recognized) or add an ignore pattern for your CSS files (e.g., exclude *.css or frontend CSS directory) so the linter skips these files; modify the project's biome.json/biome.toml to set the parser/rule for Tailwind/PostCSS or add the files ignore entry and re-run CI to verify the lint passes.
🧹 Nitpick comments (1)
frontend/src/components/Layout.tsx (1)
138-172: Duplicate toggle button markup — extract a shared component.The desktop (Lines 138–172) and mobile (Lines 206–240) toggle buttons are identical in every way except for the
mr-4margin class on the desktop version. Both carry the full moon and sun SVG paths. This is a straightforward DRY violation.♻️ Proposed refactor — extract `ThemeToggleButton`
Create
frontend/src/components/ThemeToggleButton.tsx:import { useTheme } from '../context/ThemeContext'; export default function ThemeToggleButton({ className = '' }: { className?: string }) { const { theme, toggleTheme } = useTheme(); const isDark = theme === 'dark'; return ( <button type="button" onClick={toggleTheme} aria-label={isDark ? 'Switch to light mode' : 'Switch to dark mode'} className={`inline-flex items-center justify-center w-10 h-10 rounded-full border border-transparent hover:border-dark-600/60 bg-transparent hover:bg-dark-700/40 transition-colors ${className}`} > {isDark ? ( <svg className="w-5 h-5 text-gray-200" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="1.6" strokeLinecap="round" strokeLinejoin="round" aria-hidden="true"> <path d="M21 14.5a8.5 8.5 0 01-11.5-11.5 8.5 8.5 0 1011.5 11.5z" /> </svg> ) : ( <svg className="w-5 h-5 text-gray-700" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="1.6" strokeLinecap="round" strokeLinejoin="round" aria-hidden="true"> <circle cx="12" cy="12" r="4" /> <path d="M12 2v2m0 16v2M20 12h2M2 12h2M17.657 6.343l-1.414 1.414M7.757 16.243l-1.414 1.414M6.343 6.343l1.414 1.414M16.243 16.243l1.414 1.414" /> </svg> )} </button> ); }Then in
Layout.tsx:- import { useTheme } from '../context/ThemeContext'; + import ThemeToggleButton from './ThemeToggleButton'; // Desktop - <button ... className="... mr-4"> - {/* full icon JSX */} - </button> + <ThemeToggleButton className="mr-4" /> // Mobile - <button ... className="..."> - {/* full icon JSX */} - </button> + <ThemeToggleButton />Also applies to: 206-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Layout.tsx` around lines 138 - 172, The theme toggle button markup is duplicated; extract a new ThemeToggleButton component that reads theme and toggleTheme (same logic using isDark) and renders the shared button + SVGs, accept a className prop to allow the desktop variant to pass "mr-4", then replace both inline buttons in Layout (the desktop and mobile render sites) with ThemeToggleButton (imported into Layout) passing className="mr-4" for the desktop instance and nothing for mobile so behavior and visuals remain identical but DRY is restored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/SearchBar.tsx`:
- Around line 238-244: The inline casts using (r as any) and duplicated
branching should be replaced by calling the existing helper getPrimaryText(r)
inside the div to remove no-explicit-any violations and reuse the unused
function; update the JSX at the location rendering primary text to render
{getPrimaryText(r)} (ensuring getPrimaryText is in scope) and remove the
duplicated conditional expressions/casts so the helper handles all types (block,
transaction, address, nft, nft_collection).
In `@frontend/src/context/ThemeContext.tsx`:
- Around line 13-14: Split the hook out of the component file: remove the
exported useTheme hook from ThemeContext and place it in its own module that
imports ThemeContext and calls useContext(ThemeContext), throwing if undefined;
keep ThemeProvider (the component that uses ThemeContext) in the original file
and export only components from that file to satisfy
react-refresh/only-export-components. Update exports so ThemeContext file
exports ThemeProvider and ThemeContext, the new module exports useTheme, and
update any imports across the codebase to import useTheme from the new module
and ThemeProvider/ThemeContext from the original module.
---
Outside diff comments:
In `@frontend/src/components/Layout.tsx`:
- Line 84: The useEffect dependency list omits displayedHeight causing an eslint
react-hooks/exhaustive-deps warning; fix by reading displayedRef.current instead
of the displayedHeight state inside the effect (so the effect no longer depends
on displayedHeight) and then remove displayedHeight from the dependency array
(leave [height, bps]); update references inside the effect to use
displayedRef.current where displayedHeight was used; alternatively, if you must
keep the state read, add a single-line suppression comment above the effect (//
eslint-disable-next-line react-hooks/exhaustive-deps) — target the effect in
Layout component where displayedHeight and displayedRef are used.
In `@frontend/src/pages/BlocksPage.tsx`:
- Line 26: The declared state variable `tick` in BlocksPage.tsx is unused
causing a lint failure; update the useState destructuring to discard the value
(e.g., replace `const [tick, setTick] = useState(0)` with a form that omits the
first element) so you keep `setTick` for the re-render side effect but remove
the unused `tick` identifier.
In `@frontend/src/pages/NFTTokenPage.tsx`:
- Around line 109-165: The Details panel uses non-semantic text-gray-200 on
several <dd> elements; replace those with the theme-aware semantic class (e.g.
text-fg) so values match the label styling: update the span with class "hash
text-gray-200", the fallback <span className="text-gray-200">---</span>
instances, and the <dd className="text-gray-200"> for Last Transfer to use
text-fg (or text-fg-subtle if you want lower emphasis) so all token-related
outputs (token?.token_id/tokenId, contract/contractAddress fallbacks, owner
fallback, and token.last_transfer_block display) follow the semantic color
system.
---
Duplicate comments:
In `@frontend/src/index.css`:
- Around line 57-59: Biome is flagging the Tailwind `@apply` usage in the CSS rule
"body { `@apply` font-sans antialiased transition-colors duration-200; }"; update
the Biome configuration to either enable Tailwind/PostCSS parsing (so `@apply` is
recognized) or add an ignore pattern for your CSS files (e.g., exclude *.css or
frontend CSS directory) so the linter skips these files; modify the project's
biome.json/biome.toml to set the parser/rule for Tailwind/PostCSS or add the
files ignore entry and re-run CI to verify the lint passes.
---
Nitpick comments:
In `@frontend/src/components/Layout.tsx`:
- Around line 138-172: The theme toggle button markup is duplicated; extract a
new ThemeToggleButton component that reads theme and toggleTheme (same logic
using isDark) and renders the shared button + SVGs, accept a className prop to
allow the desktop variant to pass "mr-4", then replace both inline buttons in
Layout (the desktop and mobile render sites) with ThemeToggleButton (imported
into Layout) passing className="mr-4" for the desktop instance and nothing for
mobile so behavior and visuals remain identical but DRY is restored.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
frontend/src/App.tsxfrontend/src/components/ContractTypeBadge.tsxfrontend/src/components/CopyButton.tsxfrontend/src/components/EventLogs.tsxfrontend/src/components/Layout.tsxfrontend/src/components/Pagination.tsxfrontend/src/components/ProxyBadge.tsxfrontend/src/components/SearchBar.tsxfrontend/src/components/StatusBadge.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/index.cssfrontend/src/pages/AddressPage.tsxfrontend/src/pages/AddressesPage.tsxfrontend/src/pages/BlockDetailPage.tsxfrontend/src/pages/BlockTransactionsPage.tsxfrontend/src/pages/BlocksPage.tsxfrontend/src/pages/NFTContractPage.tsxfrontend/src/pages/NFTTokenPage.tsxfrontend/src/pages/NFTsPage.tsxfrontend/src/pages/NotFoundPage.tsxfrontend/src/pages/SearchResultsPage.tsxfrontend/src/pages/TokenDetailPage.tsxfrontend/src/pages/TokensPage.tsxfrontend/src/pages/TransactionDetailPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/src/pages/WelcomePage.tsxfrontend/tailwind.config.js
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/App.tsx
- frontend/src/pages/TransactionsPage.tsx
- frontend/src/pages/TokensPage.tsx
- frontend/src/components/StatusBadge.tsx
- frontend/src/pages/AddressPage.tsx
pthmas
force-pushed
the
pierrick/light-mode
branch
from
82440c9 to
6240637
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/NFTTokenPage.tsx (1)
109-166:⚠️ Potential issue | 🟠 Major
<dd>values in the Details panel still usetext-gray-200, which breaks light mode.The
<dt>labels were correctly updated totext-fg-subtle, but the corresponding<dd>values on lines 114 (text-gray-200for Token ID), 127 (text-gray-200fallback for Contract), 140 (text-gray-200fallback for Owner), and 146 (text-gray-200for Last Transfer) are unchanged.text-gray-200is effectively near-white (#e5e7ebin default Tailwind), which will be invisible or very low-contrast on a light card background.🎨 Proposed fix — align dd values with themed tokens
- <span className="hash text-gray-200">{token?.token_id || tokenId || '---'}</span> + <span className="hash text-fg">{token?.token_id || tokenId || '---'}</span> - <span className="text-gray-200">---</span> {/* contract fallback */} + <span className="text-fg">---</span> - <span className="text-gray-200">---</span> {/* owner fallback */} + <span className="text-fg">---</span> - <dd className="text-gray-200"> + <dd className="text-fg">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/NFTTokenPage.tsx` around lines 109 - 166, The Details panel uses literal Tailwind "text-gray-200" on several <dd> elements (Token ID token?.token_id, Contract fallback, Owner fallback, Last Transfer) which breaks light mode; update those classNames to use the themed token (e.g. replace "text-gray-200" with "text-fg") so values follow the app theme—edit the <dd> nodes in NFTTokenPage (the Token ID, Contract, Owner, Last Transfer blocks) to use "text-fg" (or another appropriate themed token) instead of "text-gray-200".
♻️ Duplicate comments (1)
frontend/src/context/ThemeContext.tsx (1)
13-50:⚠️ Potential issue | 🟠 MajorCI failure: react-refresh/only-export-components.
This file exports both a component and a hook, which violates the rule and fails CI. SplituseThemeinto its own module and exportThemeContextfrom here.✅ Suggested fix
-const ThemeContext = createContext<ThemeContextValue | undefined>(undefined); +export const ThemeContext = createContext<ThemeContextValue | undefined>(undefined); @@ -export const useTheme = () => { - const context = useContext(ThemeContext); - if (!context) { - throw new Error('useTheme must be used within a ThemeProvider'); - } - return context; -};Create
frontend/src/context/useTheme.ts:import { useContext } from 'react'; import { ThemeContext } from './ThemeContext'; export const useTheme = () => { const context = useContext(ThemeContext); if (!context) { throw new Error('useTheme must be used within a ThemeProvider'); } return context; };#!/bin/bash # Locate all imports/usages of useTheme to update after moving it. rg -n "useTheme" frontend/src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/context/ThemeContext.tsx` around lines 13 - 50, The file currently exports both a React component (ThemeProvider) and a hook (useTheme), triggering the react-refresh/only-export-components CI rule; move the hook into its own module by removing the useTheme export from this file, export ThemeContext (createContext) and ThemeProvider here, and create a new frontend/src/context/useTheme.ts that imports ThemeContext and re-implements the hook (useTheme) using useContext, throwing the same error if context is undefined; after moving, update all imports/usages of useTheme across the codebase to import from ./context/useTheme while keeping ThemeProvider and ThemeContext exports intact in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/index.css`:
- Around line 100-110: The .badge-chip rule uses an undefined variable
--color-dark-600 for background-color; update the declaration in the .badge-chip
CSS (the background-color property) to use an existing surface token (e.g.,
rgb(var(--color-surface-500)) or another defined --color-surface-* token) so the
badge renders correctly, and keep the existing border color
rgb(var(--color-surface-500)) and text color unchanged.
---
Outside diff comments:
In `@frontend/src/pages/NFTTokenPage.tsx`:
- Around line 109-166: The Details panel uses literal Tailwind "text-gray-200"
on several <dd> elements (Token ID token?.token_id, Contract fallback, Owner
fallback, Last Transfer) which breaks light mode; update those classNames to use
the themed token (e.g. replace "text-gray-200" with "text-fg") so values follow
the app theme—edit the <dd> nodes in NFTTokenPage (the Token ID, Contract,
Owner, Last Transfer blocks) to use "text-fg" (or another appropriate themed
token) instead of "text-gray-200".
---
Duplicate comments:
In `@frontend/src/context/ThemeContext.tsx`:
- Around line 13-50: The file currently exports both a React component
(ThemeProvider) and a hook (useTheme), triggering the
react-refresh/only-export-components CI rule; move the hook into its own module
by removing the useTheme export from this file, export ThemeContext
(createContext) and ThemeProvider here, and create a new
frontend/src/context/useTheme.ts that imports ThemeContext and re-implements the
hook (useTheme) using useContext, throwing the same error if context is
undefined; after moving, update all imports/usages of useTheme across the
codebase to import from ./context/useTheme while keeping ThemeProvider and
ThemeContext exports intact in this file.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
frontend/src/App.tsxfrontend/src/components/ContractTypeBadge.tsxfrontend/src/components/CopyButton.tsxfrontend/src/components/EventLogs.tsxfrontend/src/components/Layout.tsxfrontend/src/components/Pagination.tsxfrontend/src/components/ProxyBadge.tsxfrontend/src/components/SearchBar.tsxfrontend/src/components/StatusBadge.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/index.cssfrontend/src/pages/AddressPage.tsxfrontend/src/pages/AddressesPage.tsxfrontend/src/pages/BlockDetailPage.tsxfrontend/src/pages/BlockTransactionsPage.tsxfrontend/src/pages/BlocksPage.tsxfrontend/src/pages/NFTContractPage.tsxfrontend/src/pages/NFTTokenPage.tsxfrontend/src/pages/NFTsPage.tsxfrontend/src/pages/NotFoundPage.tsxfrontend/src/pages/SearchResultsPage.tsxfrontend/src/pages/TokenDetailPage.tsxfrontend/src/pages/TokensPage.tsxfrontend/src/pages/TransactionDetailPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/src/pages/WelcomePage.tsxfrontend/tailwind.config.js
🚧 Files skipped from review as they are similar to previous changes (15)
- frontend/src/pages/TokensPage.tsx
- frontend/src/pages/NFTsPage.tsx
- frontend/src/pages/TransactionDetailPage.tsx
- frontend/src/pages/NotFoundPage.tsx
- frontend/src/components/Pagination.tsx
- frontend/src/components/SearchBar.tsx
- frontend/src/components/Layout.tsx
- frontend/src/pages/BlockDetailPage.tsx
- frontend/src/pages/TokenDetailPage.tsx
- frontend/src/pages/AddressesPage.tsx
- frontend/src/pages/SearchResultsPage.tsx
- frontend/src/pages/TransactionsPage.tsx
- frontend/src/components/ContractTypeBadge.tsx
- frontend/src/pages/BlockTransactionsPage.tsx
- frontend/src/pages/WelcomePage.tsx
pthmas
force-pushed
the
pierrick/light-mode
branch
from
6240637 to
c161e4a
Compare
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 (2)
frontend/src/hooks/useProxies.ts (1)
20-45:⚠️ Potential issue | 🟠 MajorAvoid refetch loop caused by
paramsobject identity.Depending on the whole
paramsobject means callers using inline literals (e.g.,useProxies({ page, limit })) will triggeruseEffecton every render; the hook’s own state updates then cause repeated fetches. Stabilize the dependencies (e.g., by destructuring the fields you actually use) to prevent identity churn.🔧 Suggested fix (stabilize by fields)
export function useProxies(params: GetProxiesParams = {}): UseProxiesResult { + const { page = 1, limit = 20 } = params; const [proxies, setProxies] = useState<ProxyInfo[]>([]); const [pagination, setPagination] = useState<{ page: number; limit: number; total: number; total_pages: number } | null>(null); const [loading, setLoading] = useState(true); const [error, setError] = useState<ApiError | null>(null); - const paramsRef = useRef(params); - paramsRef.current = params; + const paramsRef = useRef<GetProxiesParams>({ page, limit }); + paramsRef.current = { page, limit }; const fetchProxies = useCallback(async () => { setLoading(true); setError(null); try { const response = await getProxies(paramsRef.current); @@ - }, []); + }, []); useEffect(() => { fetchProxies(); - }, [fetchProxies, params]); + }, [fetchProxies, page, limit]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useProxies.ts` around lines 20 - 45, The hook currently depends on the whole params object which causes identity churn and refetch loops; update useProxies to stabilize dependencies by destructuring the specific fields you actually need from params (e.g., const { page, limit, filter } = params) and use those primitives in the useCallback for fetchProxies and in the useEffect dependency array instead of params, or alternatively memoize params upstream (useMemo) and include that memoized object in dependencies; ensure paramsRef usage is removed or updated so fetchProxies reads the stable fields (refer to paramsRef, fetchProxies, and the useEffect that calls fetchProxies) and adjust setPagination/setProxies logic unchanged.frontend/src/pages/AddressPage.tsx (1)
95-103:⚠️ Potential issue | 🟡 MinorAlign NFT label between header and overview.
The overview now says “NFT Collection” while the header still says “NFT Contract,” which reads inconsistent for the same address type.
Suggested tweak
- ? 'NFT Contract' + ? 'NFT Collection'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AddressPage.tsx` around lines 95 - 103, The header text in AddressPage.tsx is inconsistent with the overview for NFT addresses; update the conditional in the h1 render (the JSX block that checks address?.address_type) so that when address?.address_type === 'nft' it shows "NFT Collection" instead of "NFT Contract" (leave the other branches for 'erc20', 'contract', and default "Address" unchanged) to align the header with the overview.
♻️ Duplicate comments (2)
frontend/src/index.css (2)
100-109:⚠️ Potential issue | 🟡 MinorUse a defined token for
.badge-chipbackground.
--color-dark-600isn’t defined in the token set, so the background can render incorrectly.🛠️ Suggested fix
- background-color: rgb(var(--color-dark-600)); + background-color: rgb(var(--color-surface-600));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 100 - 109, The .badge-chip rule uses an undefined token --color-dark-600 for background-color; either replace that value with an existing design token (for example use rgb(var(--color-surface-600)) or another defined --color-... token used across the project) in the .badge-chip selector, or if the darker token is intentional add and define --color-dark-600 in your global token file; update the background-color declaration in frontend/src/index.css accordingly.
56-59:⚠️ Potential issue | 🟡 MinorResolve Biome Tailwind parsing errors for
@apply.
The@applydirectives in thebodyand.btn-secondaryblocks still trigger Biome parse errors. Enable Tailwind CSS parsing in Biome or configure it to ignore Tailwind syntax in CSS.#!/bin/bash # Locate Biome config and inspect CSS/Tailwind settings. fd -I 'biome\.json|biome\.config\.(js|cjs|ts|json)' rg -n 'tailwind|css' $(fd -I 'biome\.json|biome\.config\.(js|cjs|ts|json)')Also applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 56 - 59, The Biome parser is choking on Tailwind's `@apply` in frontend/src/index.css (the body and .btn-secondary blocks using `@apply`); fix by updating your Biome configuration so Tailwind CSS syntax is either parsed or ignored for this file: either enable Tailwind support/plug-in in your biome.config.(js|json) (so `@apply` is recognized) or add an exclusion for frontend/src/index.css (or disable CSS linting) so Biome skips Tailwind-specific directives; ensure the changes target the config entry that controls CSS parsing/plugins so the body and .btn-secondary `@apply` rules no longer produce parse errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/useAddresses.ts`:
- Around line 35-37: The effect currently lists params in its dependency array
causing unwanted retriggers despite the hook intentionally storing params in
paramsRef and using paramsRef.current in fetchAddresses; remove params from the
useEffect dependency array (leave [fetchAddresses]) so the effect only depends
on the stable fetchAddresses, or alternatively enforce callers to stabilize
params (e.g., memoize) and document that requirement — update the useEffect call
that references fetchAddresses and paramsRef to only depend on fetchAddresses
and ensure fetchAddresses reads params from paramsRef.current.
In `@frontend/src/hooks/useLogs.ts`:
- Around line 51-53: The useEffect hooks calling fetchLogs currently include
params in their dependency arrays causing unnecessary refetch loops; remove
params from those dependencies so the effects depend only on [fetchLogs], and
rely on paramsRef inside fetchLogs to read current params (adjust the other two
useEffect occurrences that reference params similarly), ensuring fetchLogs
remains dependent only on the stable identifier (txHash or address).
In `@frontend/src/hooks/useNFTs.ts`:
- Around line 41-43: The effect dependency list is causing unnecessary
retriggers because callers may pass non-memoized params; use the existing
paramsRef to read the latest params and remove raw params from useEffect
dependencies. Update each useEffect that currently lists [fetchContracts,
params] (and other occurrences noted) to depend only on memoized callbacks like
fetchContracts (or other stable functions), and ensure the hook reads params via
paramsRef.current inside those effects; alternatively, if you prefer requiring
callers to memoize, document that callers must pass a stable/memoized params
object and keep params in the dependency arrays—pick one approach and apply it
consistently in useNFTs.ts (functions/identifiers to change: useEffect usages,
fetchContracts, paramsRef, and any other useEffect referencing params).
In `@frontend/src/hooks/useTokens.ts`:
- Around line 53-55: The effect is re-running due to including the transient
params object in dependencies; update the useEffect calls (the one containing
useEffect(() => { fetchTokens(); }, [fetchTokens, params])) to remove params
from the dependency array and depend only on fetchTokens (which already reads
current values from paramsRef), or alternatively ensure callers pass a memoized
params; specifically modify the effects at useEffect sites referenced (lines
with useEffect that call fetchTokens) to use [fetchTokens] instead of
[fetchTokens, params], relying on paramsRef inside fetchTokens to access current
page/limit.
In `@frontend/src/hooks/useTransactions.ts`:
- Around line 42-44: useTransactions and useAddressTransactions currently
include params in their useEffect dependency arrays which causes refetch loops
when callers pass inline objects; remove params from the dependency arrays so
they read useEffect(() => { fetchTransactions(); }, [fetchTransactions]) (or
alternatively enforce callers to pass memoized params via useMemo), and ensure
you keep using the existing paramsRef for reading the current params inside
fetchTransactions to avoid stale values; update both useTransactions and
useAddressTransactions to reference only fetchTransactions in the dependency
list.
---
Outside diff comments:
In `@frontend/src/hooks/useProxies.ts`:
- Around line 20-45: The hook currently depends on the whole params object which
causes identity churn and refetch loops; update useProxies to stabilize
dependencies by destructuring the specific fields you actually need from params
(e.g., const { page, limit, filter } = params) and use those primitives in the
useCallback for fetchProxies and in the useEffect dependency array instead of
params, or alternatively memoize params upstream (useMemo) and include that
memoized object in dependencies; ensure paramsRef usage is removed or updated so
fetchProxies reads the stable fields (refer to paramsRef, fetchProxies, and the
useEffect that calls fetchProxies) and adjust setPagination/setProxies logic
unchanged.
In `@frontend/src/pages/AddressPage.tsx`:
- Around line 95-103: The header text in AddressPage.tsx is inconsistent with
the overview for NFT addresses; update the conditional in the h1 render (the JSX
block that checks address?.address_type) so that when address?.address_type ===
'nft' it shows "NFT Collection" instead of "NFT Contract" (leave the other
branches for 'erc20', 'contract', and default "Address" unchanged) to align the
header with the overview.
---
Duplicate comments:
In `@frontend/src/index.css`:
- Around line 100-109: The .badge-chip rule uses an undefined token
--color-dark-600 for background-color; either replace that value with an
existing design token (for example use rgb(var(--color-surface-600)) or another
defined --color-... token used across the project) in the .badge-chip selector,
or if the darker token is intentional add and define --color-dark-600 in your
global token file; update the background-color declaration in
frontend/src/index.css accordingly.
- Around line 56-59: The Biome parser is choking on Tailwind's `@apply` in
frontend/src/index.css (the body and .btn-secondary blocks using `@apply`); fix by
updating your Biome configuration so Tailwind CSS syntax is either parsed or
ignored for this file: either enable Tailwind support/plug-in in your
biome.config.(js|json) (so `@apply` is recognized) or add an exclusion for
frontend/src/index.css (or disable CSS linting) so Biome skips Tailwind-specific
directives; ensure the changes target the config entry that controls CSS
parsing/plugins so the body and .btn-secondary `@apply` rules no longer produce
parse errors.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
frontend/src/App.tsxfrontend/src/components/ContractTypeBadge.tsxfrontend/src/components/CopyButton.tsxfrontend/src/components/EventLogs.tsxfrontend/src/components/Layout.tsxfrontend/src/components/Pagination.tsxfrontend/src/components/ProxyBadge.tsxfrontend/src/components/SearchBar.tsxfrontend/src/components/StatusBadge.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/context/theme-context.tsfrontend/src/hooks/useAddresses.tsfrontend/src/hooks/useBlocks.tsfrontend/src/hooks/useEthPrice.tsfrontend/src/hooks/useLogs.tsfrontend/src/hooks/useNFTs.tsfrontend/src/hooks/useProxies.tsfrontend/src/hooks/useTheme.tsfrontend/src/hooks/useTokens.tsfrontend/src/hooks/useTransactions.tsfrontend/src/hooks/useTransfers.tsfrontend/src/index.cssfrontend/src/pages/AddressPage.tsxfrontend/src/pages/AddressesPage.tsxfrontend/src/pages/BlockDetailPage.tsxfrontend/src/pages/BlockTransactionsPage.tsxfrontend/src/pages/BlocksPage.tsxfrontend/src/pages/NFTContractPage.tsxfrontend/src/pages/NFTTokenPage.tsxfrontend/src/pages/NFTsPage.tsxfrontend/src/pages/NotFoundPage.tsxfrontend/src/pages/SearchResultsPage.tsxfrontend/src/pages/TokenDetailPage.tsxfrontend/src/pages/TokensPage.tsxfrontend/src/pages/TransactionDetailPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/src/pages/WelcomePage.tsxfrontend/tailwind.config.js
✅ Files skipped from review due to trivial changes (1)
- frontend/src/pages/NFTContractPage.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- frontend/src/pages/SearchResultsPage.tsx
- frontend/src/pages/BlockDetailPage.tsx
- frontend/src/pages/BlockTransactionsPage.tsx
- frontend/src/pages/BlocksPage.tsx
- frontend/src/pages/TransactionDetailPage.tsx
- frontend/src/pages/WelcomePage.tsx
- frontend/src/components/Pagination.tsx
- frontend/src/pages/NFTTokenPage.tsx
- frontend/src/components/EventLogs.tsx
- frontend/src/pages/NotFoundPage.tsx
- frontend/src/pages/TokenDetailPage.tsx
- frontend/src/pages/TransactionsPage.tsx
- frontend/src/components/SearchBar.tsx
- frontend/src/components/Layout.tsx
- frontend/src/components/ProxyBadge.tsx
- frontend/src/pages/AddressesPage.tsx
- frontend/src/App.tsx
pthmas
force-pushed
the
pierrick/light-mode
branch
from
c161e4a to
18a032f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ProxyBadge.tsx (1)
49-51:⚠️ Potential issue | 🟡 MinorReplace hardcoded gray text utilities with theme-aware
text-fg-mutedtoken to ensure proper contrast in light mode.
text-gray-400andtext-gray-500are unconditional values that don't adapt to theme changes. Your Tailwind config and index.css define a theme-awaretext-fg-mutedtoken that adjusts for both dark (rgb(203 213 225)) and light (rgb(88 73 63)) modes, maintaining visual hierarchy across both themes.Apply these replacements:
- Line 49:
text-gray-400→text-fg-muted- Line 55:
text-gray-500→text-fg-mutedDiff
<span className="text-gray-400 text-xs"> + <span className="text-fg-muted text-xs"> {typeLabel} </span>- <span className="text-gray-500">Implementation:</span> + <span className="text-fg-muted">Implementation:</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ProxyBadge.tsx` around lines 49 - 51, Replace hardcoded Tailwind color utilities in the ProxyBadge component with the theme-aware token: in the JSX span that renders {typeLabel} (the span using className="text-gray-400 text-xs"), change the class to use text-fg-muted instead of text-gray-400; likewise find the other span using text-gray-500 (the muted secondary text in this component) and replace text-gray-500 with text-fg-muted so both places use the theme-aware token that adapts for light/dark modes.
♻️ Duplicate comments (3)
frontend/src/hooks/useLogs.ts (1)
24-29: Params-ref pattern correctly addresses the prior review feedback.The previous review flagged
params(by reference) in useEffect deps causing re-fetch loops. The new approach — syncing to a ref and driving re-fetches viaJSON.stringify(params)key — correctly triggers fetches only when param content changes, not on every render. Consistent across all three hooks.Also applies to: 76-81, 128-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useLogs.ts` around lines 24 - 29, The params-ref pattern is correct: keep the useRef(params) as paramsRef, update paramsRef.current in the useEffect that depends on [params], and continue to drive re-fetches using the content-key txLogsParamsKey = JSON.stringify(params) so fetches only trigger on param content changes; apply the identical change to the other two hook locations that mirror this logic (the blocks around the other useEffect instances at the noted duplicate spots) and ensure the same identifiers (paramsRef, txLogsParamsKey, and the params-sync useEffect) are used consistently.frontend/src/index.css (2)
100-110:⚠️ Potential issue | 🟡 MinorUse a defined surface token for
.badge-chipbackground.
--color-dark-600isn’t defined, so the background can render incorrectly.🛠️ Suggested fix
- background-color: rgb(var(--color-dark-600)); + background-color: rgb(var(--color-surface-600));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 100 - 110, The .badge-chip rule uses an undefined variable (--color-dark-600) for background-color; update the CSS to use an existing surface token instead (for example replace background-color: rgb(var(--color-dark-600)) with background-color: rgb(var(--color-surface-600)) or another defined --color-surface-* token) so the badge renders consistently; locate the .badge-chip selector in frontend/src/index.css and change only the background-color declaration to reference a defined token.
57-59:⚠️ Potential issue | 🟡 MinorBiome still flags Tailwind
@apply; confirm CSS parser support.
If CI uses Biome, ensure Tailwind CSS syntax is enabled or excluded for these rules.#!/bin/bash # Locate Biome config and inspect CSS/Tailwind settings. fd -a '^biome\.(json|jsonc|toml)$' . for f in $(fd -a '^biome\.(json|jsonc|toml)$'); do echo "---- $f ----" sed -n '1,200p' "$f" doneAlso applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 57 - 59, The Tailwind `@apply` usage in frontend/src/index.css (the rule: body { `@apply` font-sans antialiased transition-colors duration-200; }) is being flagged by Biome; update your Biome config (biome.json/biome.jsonc/biome.toml) to either enable Tailwind CSS syntax/plugin support or add an ignore pattern for this CSS file (or the CSS syntax rules) so Biome won’t error on `@apply`; locate the project's biome.* config and add the Tailwind plugin/syntax setting or include the file path in ignoreFiles/exclude patterns, then re-run CI to verify the warning is resolved.
🧹 Nitpick comments (2)
frontend/tailwind.config.js (1)
21-34:darkandsurfacegroups are identical — intentional backward-compat alias?Both map to the same
--color-surface-*variables. If this is a migration bridge so existingbg-dark-*classes keep working while introducing semanticbg-surface-*tokens, consider adding a brief comment to clarify intent, so a future contributor doesn't remove one thinking it's duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/tailwind.config.js` around lines 21 - 34, The dark and surface theme color groups in tailwind.config.js are identical (the dark and surface objects both map 900..500 to --color-surface-*) and should be documented as an intentional alias if that’s the case; update the tailwind config near the dark and surface objects (and reference the withOpacityValue usage and the --color-surface-* variables) to add a concise comment explaining that dark is a backward-compatibility alias for surface (or consolidate if not intended), so future maintainers understand why both groups exist.frontend/src/components/Layout.tsx (1)
138-172: Consider extracting the theme toggle into a shared component.The desktop (Lines 138–172) and mobile (Lines 206–240) toggle buttons are identical markup. Extracting a
ThemeTogglecomponent would eliminate the duplication and make future icon or behavior changes a single-point edit.Sketch
function ThemeToggle({ className }: { className?: string }) { const { theme, toggleTheme } = useTheme(); const isDark = theme === 'dark'; return ( <button type="button" onClick={toggleTheme} aria-label={isDark ? 'Switch to light mode' : 'Switch to dark mode'} className={`inline-flex items-center justify-center w-10 h-10 rounded-full border border-transparent hover:border-dark-600/60 bg-transparent hover:bg-dark-700/40 transition-colors ${className ?? ''}`} > {isDark ? (/* moon SVG */) : (/* sun SVG */)} </button> ); }Also applies to: 206-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Layout.tsx` around lines 138 - 172, Duplicate theme toggle markup should be extracted into a reusable ThemeToggle component: create a ThemeToggle({ className?: string }) component that calls useTheme(), derives isDark from theme, and renders the existing button (including aria-label, onClick calling toggleTheme, and both moon/sun SVGs) while applying the passed className; then replace the two identical button instances in the Layout component with <ThemeToggle className="..."/> (preserving their current class strings such as "inline-flex ... mr-4" and the mobile variant) so future icon/behavior changes are centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Layout.tsx`:
- Line 84: The effect driving the requestAnimationFrame loop should not include
displayedHeight in its dependency array because setDisplayedHeight updates that
state each frame and causes the effect to teardown/restart (resetting
lastFrameRef.current), creating a micro-stutter; remove displayedHeight from the
deps so the effect depends only on height and bps, relying on
displayedRef.current for per-frame reads/writes, and keep the existing cleanup
that cancels the rAF and resets lastFrameRef.current.
In `@frontend/tailwind.config.js`:
- Around line 1-6: The withOpacityValue helper (withOpacityValue) needs to
support Tailwind v3.4.19 opacity semantics: either replace the function entirely
with the simpler string form using the "<alpha-value>" placeholder for CSS vars,
or update the function signature to accept and handle both opacityValue and
opacityVariable (so it returns the var(...) form when opacityVariable is set and
the slash form when opacityValue is provided). Locate withOpacityValue and
implement one of these two fixes so Tailwind's opacity utilities work with both
modern "<alpha-value>" usage and the older opacityVariable pattern.
---
Outside diff comments:
In `@frontend/src/components/ProxyBadge.tsx`:
- Around line 49-51: Replace hardcoded Tailwind color utilities in the
ProxyBadge component with the theme-aware token: in the JSX span that renders
{typeLabel} (the span using className="text-gray-400 text-xs"), change the class
to use text-fg-muted instead of text-gray-400; likewise find the other span
using text-gray-500 (the muted secondary text in this component) and replace
text-gray-500 with text-fg-muted so both places use the theme-aware token that
adapts for light/dark modes.
---
Duplicate comments:
In `@frontend/src/hooks/useLogs.ts`:
- Around line 24-29: The params-ref pattern is correct: keep the useRef(params)
as paramsRef, update paramsRef.current in the useEffect that depends on
[params], and continue to drive re-fetches using the content-key txLogsParamsKey
= JSON.stringify(params) so fetches only trigger on param content changes; apply
the identical change to the other two hook locations that mirror this logic (the
blocks around the other useEffect instances at the noted duplicate spots) and
ensure the same identifiers (paramsRef, txLogsParamsKey, and the params-sync
useEffect) are used consistently.
In `@frontend/src/index.css`:
- Around line 100-110: The .badge-chip rule uses an undefined variable
(--color-dark-600) for background-color; update the CSS to use an existing
surface token instead (for example replace background-color:
rgb(var(--color-dark-600)) with background-color: rgb(var(--color-surface-600))
or another defined --color-surface-* token) so the badge renders consistently;
locate the .badge-chip selector in frontend/src/index.css and change only the
background-color declaration to reference a defined token.
- Around line 57-59: The Tailwind `@apply` usage in frontend/src/index.css (the
rule: body { `@apply` font-sans antialiased transition-colors duration-200; }) is
being flagged by Biome; update your Biome config
(biome.json/biome.jsonc/biome.toml) to either enable Tailwind CSS syntax/plugin
support or add an ignore pattern for this CSS file (or the CSS syntax rules) so
Biome won’t error on `@apply`; locate the project's biome.* config and add the
Tailwind plugin/syntax setting or include the file path in ignoreFiles/exclude
patterns, then re-run CI to verify the warning is resolved.
---
Nitpick comments:
In `@frontend/src/components/Layout.tsx`:
- Around line 138-172: Duplicate theme toggle markup should be extracted into a
reusable ThemeToggle component: create a ThemeToggle({ className?: string })
component that calls useTheme(), derives isDark from theme, and renders the
existing button (including aria-label, onClick calling toggleTheme, and both
moon/sun SVGs) while applying the passed className; then replace the two
identical button instances in the Layout component with <ThemeToggle
className="..."/> (preserving their current class strings such as "inline-flex
... mr-4" and the mobile variant) so future icon/behavior changes are
centralized.
In `@frontend/tailwind.config.js`:
- Around line 21-34: The dark and surface theme color groups in
tailwind.config.js are identical (the dark and surface objects both map 900..500
to --color-surface-*) and should be documented as an intentional alias if that’s
the case; update the tailwind config near the dark and surface objects (and
reference the withOpacityValue usage and the --color-surface-* variables) to add
a concise comment explaining that dark is a backward-compatibility alias for
surface (or consolidate if not intended), so future maintainers understand why
both groups exist.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
frontend/src/App.tsxfrontend/src/components/ContractTypeBadge.tsxfrontend/src/components/CopyButton.tsxfrontend/src/components/EventLogs.tsxfrontend/src/components/Layout.tsxfrontend/src/components/Pagination.tsxfrontend/src/components/ProxyBadge.tsxfrontend/src/components/SearchBar.tsxfrontend/src/components/StatusBadge.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/context/theme-context.tsfrontend/src/hooks/useAddresses.tsfrontend/src/hooks/useBlocks.tsfrontend/src/hooks/useEthPrice.tsfrontend/src/hooks/useLogs.tsfrontend/src/hooks/useNFTs.tsfrontend/src/hooks/useProxies.tsfrontend/src/hooks/useTheme.tsfrontend/src/hooks/useTokens.tsfrontend/src/hooks/useTransactions.tsfrontend/src/hooks/useTransfers.tsfrontend/src/index.cssfrontend/src/pages/AddressPage.tsxfrontend/src/pages/AddressesPage.tsxfrontend/src/pages/BlockDetailPage.tsxfrontend/src/pages/BlockTransactionsPage.tsxfrontend/src/pages/BlocksPage.tsxfrontend/src/pages/NFTContractPage.tsxfrontend/src/pages/NFTTokenPage.tsxfrontend/src/pages/NFTsPage.tsxfrontend/src/pages/NotFoundPage.tsxfrontend/src/pages/SearchResultsPage.tsxfrontend/src/pages/TokenDetailPage.tsxfrontend/src/pages/TokensPage.tsxfrontend/src/pages/TransactionDetailPage.tsxfrontend/src/pages/TransactionsPage.tsxfrontend/src/pages/WelcomePage.tsxfrontend/tailwind.config.js
🚧 Files skipped from review as they are similar to previous changes (20)
- frontend/src/pages/WelcomePage.tsx
- frontend/src/context/ThemeContext.tsx
- frontend/src/pages/SearchResultsPage.tsx
- frontend/src/hooks/useTheme.ts
- frontend/src/pages/NFTsPage.tsx
- frontend/src/components/EventLogs.tsx
- frontend/src/pages/AddressesPage.tsx
- frontend/src/pages/BlockTransactionsPage.tsx
- frontend/src/pages/AddressPage.tsx
- frontend/src/hooks/useBlocks.ts
- frontend/src/App.tsx
- frontend/src/hooks/useAddresses.ts
- frontend/src/pages/TokensPage.tsx
- frontend/src/pages/TransactionsPage.tsx
- frontend/src/pages/BlocksPage.tsx
- frontend/src/components/CopyButton.tsx
- frontend/src/pages/NFTContractPage.tsx
- frontend/src/components/ContractTypeBadge.tsx
- frontend/src/pages/TransactionDetailPage.tsx
- frontend/src/components/StatusBadge.tsx
pthmas
force-pushed
the
pierrick/light-mode
branch
from
18a032f to
59f3e6a
Compare
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
2 participants
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.
Overview
Closes #2
Summary by CodeRabbit
New Features
Style Updates