Skip to content

styling#298

Merged
ND68 merged 1 commit intomainfrom
andy/281-notification-banners
Mar 4, 2026
Merged

styling#298
ND68 merged 1 commit intomainfrom
andy/281-notification-banners

Conversation

@ND68
Copy link
Collaborator

@ND68 ND68 commented Mar 4, 2026

#281 minor styling changes

@netlify
Copy link

netlify bot commented Mar 4, 2026

Deploy Preview for hope-for-haiti ready!

Name Link
🔨 Latest commit 0904bfc
🔍 Latest deploy log https://app.netlify.com/projects/hope-for-haiti/deploys/69a8a39d9185b60008d4fd6f
😎 Deploy Preview https://deploy-preview-298--hope-for-haiti.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ND68 ND68 merged commit 2563d49 into main Mar 4, 2026
3 of 4 checks passed
@ND68 ND68 deleted the andy/281-notification-banners branch March 4, 2026 21:27
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR makes a minor styling adjustment to the global Toaster notification component in src/app/layout.tsx by adding a containerStyle prop with top and right offsets to reposition the toast container.

Issues found:

  • position / containerStyle conflict: position="bottom-right" instructs react-hot-toast to anchor the container at the bottom of the viewport, while top: 20 in containerStyle adds a conflicting top offset. The two properties fight each other on fixed-position elements and will likely push the toasts to the top of the screen rather than the bottom. The position prop should be updated to match the intended placement (e.g. "top-right"), or containerStyle should use bottom/right offsets to stay consistent with "bottom-right".
  • Hardcoded px values: Numeric values in React inline styles are treated as px (e.g. top: 2020px). Project convention requires rem units (1rem = 10px), so these should be string values like "2rem" and "8rem".

PR Checklist areas needing improvement:

  • ❌ No hardcoded sizes — inline px values used instead of rem strings

Confidence Score: 3/5

  • The position conflict may cause toasts to render in the wrong area of the screen; needs a fix before merging.
  • The change is small and scoped to a single file, but the contradiction between position="bottom-right" and top: 20 is a functional issue that will visually misplace the notification toaster. The hardcoded px values also violate project conventions.
  • src/app/layout.tsx — position prop conflicts with the added containerStyle top offset

Important Files Changed

Filename Overview
src/app/layout.tsx Added containerStyle with top: 20 and right: 80 to the Toaster component. The top offset directly conflicts with the existing position="bottom-right" prop and hardcoded px values violate the project rem convention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Toaster rendered in RootLayout] --> B{position prop}
    B -->|"bottom-right"| C[Library sets fixed bottom offset]
    A --> D{containerStyle}
    D -->|"top: 20px"| E[Browser also applies top offset]
    C & E --> F[⚠️ Conflicting top + bottom on fixed element]
    F --> G[Unpredictable toast placement]

    H[Fix Option A] --> I["Change position to 'top-right'\nKeep top: 20 / right: 80"]
    J[Fix Option B] --> K["Keep position='bottom-right'\nReplace top with bottom: 20"]
Loading

Last reviewed commit: 0904bfc

Comment on lines 27 to +32
position="bottom-right"
containerClassName="general-toaster"
containerStyle={{
top: 20,
right: 80
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflicting position and containerStyle

position="bottom-right" tells react-hot-toast to anchor the container to the bottom-right of the viewport (it sets a bottom offset internally). Adding top: 20 in containerStyle simultaneously sets a top value on the same fixed element, which fights against the bottom offset and can cause unpredictable positioning across browsers — in many cases the toast will appear near the top of the screen rather than the bottom.

If the intent is to move the toaster to the top-right, change the position prop and use only top/right offsets:

Suggested change
position="bottom-right"
containerClassName="general-toaster"
containerStyle={{
top: 20,
right: 80
}}
position="top-right"
containerClassName="general-toaster"
containerStyle={{
top: 20,
right: 80
}}

If the intent is to keep it at the bottom-right but shift it inward, use bottom/right instead:

containerStyle={{
  bottom: 20,
  right: 80
}}

Comment on lines +30 to +31
top: 20,
right: 80
Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcoded px values in inline styles

React treats bare number values in inline style objects as px, so top: 20 and right: 80 resolve to 20px and 80px. The project convention requires rem units (1rem = 10px). For inline styles, string values must be used:

Suggested change
top: 20,
right: 80
top: "2rem",
right: "8rem"

Context Used: Rule from dashboard - Use rem units instead of px for measurements like border-radius, padding, and width in CSS files. Th... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant