Skip to content

fix: change articles mobile design#3037

Open
ahtesham-quraish wants to merge 1 commit intomainfrom
ahtesham/article-mobile-view
Open

fix: change articles mobile design#3037
ahtesham-quraish wants to merge 1 commit intomainfrom
ahtesham/article-mobile-view

Conversation

@ahtesham-quraish
Copy link
Contributor

What are the relevant tickets?

Description (What does it do?)

Screenshots (if appropriate):

  • Desktop screenshots
  • Mobile width screenshots

How can this be tested?

Additional Context

Comment on lines 41 to +42
backgroundImage: backgroundDim
? `linear-gradient(rgba(0 0 0 / ${backgroundDim}%), rgba(0 0 0 / ${backgroundDim}%)), ${backgroundUrlFn}`
? `linear-gradient(rgba(0, 0, 0, ${backgroundDim}), rgba(0, 0, 0, ${backgroundDim})), ${backgroundUrlFn}`
Copy link

Choose a reason for hiding this comment

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

Bug: A CSS syntax change causes backgroundDim={30} to be interpreted as 100% opacity, rendering a fully opaque black banner on unit/channel pages.
Severity: CRITICAL

Suggested Fix

The UnitChannelTemplate.tsx component passes backgroundDim={30}. This value should be converted to the new 0-1 scale, likely 0.3, to restore the intended 30% opacity for the banner overlay.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: frontends/ol-components/src/components/Banner/Banner.tsx#L41-L42

Potential issue: A change in the CSS syntax for the `linear-gradient` background in the
`Banner` component has altered how the alpha channel is interpreted. Previously, it used
a percentage-based value (e.g., `30%`), but it now expects a float between 0 and 1. The
`UnitChannelTemplate` component passes a hardcoded `backgroundDim` value of `30`. With
the new syntax, browsers will clamp this value to `1`, resulting in a 100% opaque black
overlay. This will completely obscure the banner background image on unit and channel
pages, making the content on the banner unreadable.

Did we get this right? 👍 / 👎 to inform future reviews.

backgroundUrl = DEFAULT_BACKGROUND_IMAGE_URL,
backgroundSize = "cover",
backgroundDim = 0,
backgroundDim = 0.4,
Copy link

Choose a reason for hiding this comment

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

Bug: The default backgroundDim prop in BannerBackground was changed, unintentionally adding a 40% dark overlay to course, program, and article editor pages.
Severity: HIGH

Suggested Fix

Revert the default value of backgroundDim in BannerBackground back to 0. Alternatively, explicitly pass backgroundDim={0} to components like ProductPageTemplate and BannerNode that should not have an overlay by default.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: frontends/ol-components/src/components/Banner/Banner.tsx#L37

Potential issue: The default value for the `backgroundDim` prop in the
`BannerBackground` component was changed from `0` (no overlay) to `0.4` (40% opacity).
Components that use `BannerBackground` without explicitly providing a `backgroundDim`
prop will now unexpectedly render a dark overlay. This affects `ProductPageTemplate`,
used for all course and program pages, and `BannerNode`, used in the article editor.
These pages will now have a 40% dark overlay applied to their banners where none existed
before, causing an unintended visual change across multiple page types.

Did we get this right? 👍 / 👎 to inform future reviews.

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