Skip to content

Conversation

@nilloq
Copy link
Member

@nilloq nilloq commented Jan 5, 2026

Related issues

Fix #3298

Description

Use theme switcher icon instead of brand icon in header

Motivation & Context

Avoid displaying 2 times the brand icon in the header

Types of change

  • Bug fix (non-breaking which fixes an issue)

Live previews

Checklist

Contribution

Accessibility

  • My change follows accessibility good practices; I have at least run axe

Design

  • My change respects the design guidelines defined in Orange Design System
  • My change is compatible with a responsive display

Development

  • My change follows the developer guide
  • I have added JavaScript unit tests to cover my changes
  • I have added SCSS unit tests to cover my changes

Documentation

  • My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

  • My change introduces changes to the migration guide
  • My new component is well displayed in Storybook
  • My new component is compatible with RTL
  • Manually run BrowserStack tests
  • Manually test browser compatibility with BrowserStack (Chrome >= 60, Firefox >= 60 (+ ESR), Edge, Safari >= 12, iOS Safari, Chrome & Firefox on Android)
  • Code review
  • Design review
  • A11y review

After the merge

@netlify
Copy link

netlify bot commented Jan 5, 2026

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 300a06c
🔍 Latest deploy log https://app.netlify.com/projects/boosted/deploys/695bcfa920f4f30008e7a71f
😎 Deploy Preview https://deploy-preview-3309--boosted.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.

@nilloq nilloq marked this pull request as ready for review January 5, 2026 10:14
@boosted-bot boosted-bot moved this from In Progress / Draft to Need Dev Review in 🟣 [Orange-Boosted-Bootstrap] PRs Board Jan 5, 2026
Copy link
Collaborator

@vprothais vprothais left a comment

Choose a reason for hiding this comment

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

There is a conflict on this branch after the merge of the documentation reorganization. We should update this branch with ouds/main

aria-label={`Toggle brand (${getConfig().brand})`}
>
<img src={getVersionedDocsPath(`assets/brand/${getConfig().brand}-logo.svg`)} alt="" loading="lazy" />
<svg class:list={['bi ms-auto', { 'd-none': getConfig().brand !== 'orange' }]} aria-hidden="true">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand the the test to add d-none for other brands. We should have it on any brand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, wrong copy / paste. Fixed

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

Labels

None yet

Projects

Status: Need Dev Review

Development

Successfully merging this pull request may close these issues.

3 participants