Copilot commented on code in PR #1355:
URL: https://github.com/apache/airflow-site/pull/1355#discussion_r2649248971
##########
landing-pages/site/assets/scss/_list-boxes.scss:
##########
@@ -307,6 +326,56 @@ $card-margin: 20px;
&__integration--name {
color: var(--bs-emphasis-color);
}
+
+ // Testimonials (Transparent BG) - Consistent Gray
+ &__use-cases--testimonial--logo img {
+ filter: brightness(0) invert(0.6);
+ opacity: 1;
+ }
+
+ // Experity: Revert to original (White BG JPG)
+ .logo-experity img {
+ filter: none;
+ opacity: 0.6;
+ }
+
+ // Rancher SUSE & Sift specific: Swap logo in dark mode
+ .logo-rancherbysuse,
+ .logo-sift {
+ .logo-default { display: none; }
+ .logo-dark-mode { display: block; }
+ }
+
+ // Use Cases (White BG) - Consistent Gray via Screen Blend
+ &.hoverable-icon .box-event__use-cases--logo img {
+ // Normalize to B/W (Text=Black, BG=White) then Invert (Text=White,
BG=Black) then Gray
+ filter: brightness(0.6) contrast(25) grayscale(1) invert(1)
brightness(0.6);
+ mix-blend-mode: screen;
+ opacity: 1;
+ }
+
+ // Hover state for Use Cases (Top boxes) - Keep inverted to stay white,
but rotate hue to restore colors
+ &.hoverable-icon:hover {
+ .box-event__use-cases--logo img {
+ filter: invert(1) hue-rotate(180deg);
Review Comment:
The `hue-rotate(180deg)` filter on hover is used to restore colors after
inversion, but 180-degree hue rotation doesn't restore original colors—it
shifts them to complementary colors. For example, blue becomes yellow, red
becomes cyan. If the intent is to restore original logo colors on hover, this
approach is incorrect. Consider using separate image assets for hover states or
removing the filters entirely on hover.
```suggestion
// Hover state for Use Cases (Top boxes) - Restore original logo colors
&.hoverable-icon:hover {
.box-event__use-cases--logo img {
filter: none;
mix-blend-mode: normal;
```
##########
landing-pages/site/assets/scss/_list-boxes.scss:
##########
@@ -307,6 +326,56 @@ $card-margin: 20px;
&__integration--name {
color: var(--bs-emphasis-color);
}
+
+ // Testimonials (Transparent BG) - Consistent Gray
+ &__use-cases--testimonial--logo img {
+ filter: brightness(0) invert(0.6);
+ opacity: 1;
+ }
+
+ // Experity: Revert to original (White BG JPG)
+ .logo-experity img {
+ filter: none;
+ opacity: 0.6;
+ }
+
+ // Rancher SUSE & Sift specific: Swap logo in dark mode
+ .logo-rancherbysuse,
+ .logo-sift {
+ .logo-default { display: none; }
+ .logo-dark-mode { display: block; }
+ }
+
+ // Use Cases (White BG) - Consistent Gray via Screen Blend
+ &.hoverable-icon .box-event__use-cases--logo img {
+ // Normalize to B/W (Text=Black, BG=White) then Invert (Text=White,
BG=Black) then Gray
+ filter: brightness(0.6) contrast(25) grayscale(1) invert(1)
brightness(0.6);
+ mix-blend-mode: screen;
+ opacity: 1;
+ }
+
+ // Hover state for Use Cases (Top boxes) - Keep inverted to stay white,
but rotate hue to restore colors
+ &.hoverable-icon:hover {
+ .box-event__use-cases--logo img {
+ filter: invert(1) hue-rotate(180deg);
+ opacity: 1;
+ }
+ }
+
+ // Hover state for Testimonials - Restore original colors
+ &.hoverable-icon:hover {
+ .box-event__use-cases--testimonial--logo img {
+ filter: none;
+ opacity: 1;
+ mix-blend-mode: normal;
+ }
+ // Plarium & Onefootball specific: White on hover in dark mode
+ .logo-plarium-krasnodar img,
+ .logo-onefootball img {
+ filter: brightness(0) invert(1);
+ }
+ }
Review Comment:
The selector `&.hoverable-icon:hover` is duplicated within the
`[data-bs-theme="dark"]` block - once for Use Cases logos (lines 358-363) and
once for Testimonials (lines 366-377). While CSS will apply both rules, this
creates confusion and makes the code harder to maintain. Consider combining
these into a single hover rule or using more specific selectors to clarify the
intent.
##########
landing-pages/site/assets/scss/_list-boxes.scss:
##########
@@ -307,6 +326,56 @@ $card-margin: 20px;
&__integration--name {
color: var(--bs-emphasis-color);
}
+
+ // Testimonials (Transparent BG) - Consistent Gray
+ &__use-cases--testimonial--logo img {
+ filter: brightness(0) invert(0.6);
+ opacity: 1;
+ }
+
+ // Experity: Revert to original (White BG JPG)
+ .logo-experity img {
+ filter: none;
+ opacity: 0.6;
+ }
+
+ // Rancher SUSE & Sift specific: Swap logo in dark mode
+ .logo-rancherbysuse,
+ .logo-sift {
+ .logo-default { display: none; }
+ .logo-dark-mode { display: block; }
+ }
+
+ // Use Cases (White BG) - Consistent Gray via Screen Blend
+ &.hoverable-icon .box-event__use-cases--logo img {
+ // Normalize to B/W (Text=Black, BG=White) then Invert (Text=White,
BG=Black) then Gray
+ filter: brightness(0.6) contrast(25) grayscale(1) invert(1)
brightness(0.6);
Review Comment:
The complex filter chain `filter: brightness(0.6) contrast(25) grayscale(1)
invert(1) brightness(0.6)` may have inconsistent results across different
browsers and operating systems. Multiple stacked CSS filters can compound
rendering differences. Consider testing this thoroughly across browsers
(Chrome, Firefox, Safari, Edge) and consider using simpler filter combinations
or CSS blend modes where possible to ensure consistent visual results.
```suggestion
// Normalize to B/W (Text=Black, BG=White), then invert (Text=White,
BG=Black), then slightly dim
filter: grayscale(1) invert(1) brightness(0.6);
```
##########
landing-pages/site/layouts/partials/boxes/testimonial.html:
##########
@@ -20,9 +20,15 @@
{{ $title := .title }}
<div class="card">
<div class="box-event box-event__use-cases hoverable-icon">
- <div class="box-event__use-cases--testimonial--logo">
+ <div class="box-event__use-cases--testimonial--logo logo-{{ $title
| urlize }}">
{{ with .logo }}
- <img src="/usecase-logos/{{ . }}" alt="{{ $title }} logo" />
+ <img class="logo-default" src="/usecase-logos/{{ . }}"
alt="{{ $title }} logo" />
+ {{ end }}
+ {{ if eq $title "RancherBySUSE" }}
+ <img class="logo-dark-mode"
src="/usecase-logos/rancher-suse-white.svg" alt="{{ $title }} logo" />
+ {{ end }}
+ {{ if eq $title "Sift" }}
+ <img class="logo-dark-mode"
src="/usecase-logos/sift-logo-white.svg" alt="{{ $title }} logo" />
{{ end }}
Review Comment:
The logic for displaying alternate dark-mode logos is hardcoded for specific
company names ("RancherBySUSE" and "Sift"). This approach is not scalable and
violates the DRY principle. Consider using a data-driven approach where the
markdown frontmatter includes a `logo_dark` field, allowing any testimonial to
specify a dark mode logo variant without template changes. For example:
`logo_dark: "rancher-suse-white.svg"`
```suggestion
{{ with .logo_dark }}
<img class="logo-dark-mode" src="/usecase-logos/{{ . }}"
alt="{{ $title }} logo" />
{{ else }}
{{ if eq $title "RancherBySUSE" }}
<img class="logo-dark-mode"
src="/usecase-logos/rancher-suse-white.svg" alt="{{ $title }} logo" />
{{ else if eq $title "Sift" }}
<img class="logo-dark-mode"
src="/usecase-logos/sift-logo-white.svg" alt="{{ $title }} logo" />
{{ end }}
{{ end }}
```
##########
sphinx_airflow_theme/sphinx_airflow_theme/header.html:
##########
@@ -104,6 +70,40 @@
</div>
</div>
+ <div class="navbar__theme-toggle">
+ <button class="btn btn-link nav-link dropdown-toggle d-flex
align-items-center"
+ id="bd-theme"
+ type="button"
+ aria-expanded="false"
+ data-bs-toggle="dropdown"
+ data-bs-display="static"
+ aria-label="Toggle theme (auto)">
+ <svg class="bi my-1 theme-icon-active"><use
href="#circle-half"></use></svg>
+ </button>
+ <ul class="dropdown-menu dropdown-menu-end"
aria-labelledby="bd-theme-text">
+ <li>
+ <button type="button" class="dropdown-item d-flex
align-items-center" data-bs-theme-value="light" aria-pressed="false">
+ <svg class="bi me-2 theme-icon"><use
href="#sun-fill"></use></svg>
+ Light
+ <svg class="bi ms-auto d-none"><use
href="#check2"></use></svg>
+ </button>
+ </li>
+ <li>
+ <button type="button" class="dropdown-item d-flex
align-items-center" data-bs-theme-value="dark" aria-pressed="false">
+ <svg class="bi me-2 theme-icon"><use
href="#moon-stars-fill"></use></svg>
+ Dark
+ <svg class="bi ms-auto d-none"><use
href="#check2"></use></svg>
+ </button>
+ </li>
+ <li>
+ <button type="button" class="dropdown-item d-flex
align-items-center active" data-bs-theme-value="auto" aria-pressed="true">
+ <svg class="bi me-2 theme-icon"><use
href="#circle-half"></use></svg>
Review Comment:
The CSS class `theme-icon` is used on SVG elements but appears to be
undefined. The original code used `opacity-50` which was replaced with
`theme-icon`, but no corresponding CSS rule for `.theme-icon` exists in the
provided SCSS files. This will cause the icons to not be styled correctly,
potentially appearing at full opacity when they should be semi-transparent or
have other specific styling applied.
```suggestion
<svg class="bi me-2 opacity-50"><use
href="#sun-fill"></use></svg>
Light
<svg class="bi ms-auto d-none"><use
href="#check2"></use></svg>
</button>
</li>
<li>
<button type="button" class="dropdown-item d-flex
align-items-center" data-bs-theme-value="dark" aria-pressed="false">
<svg class="bi me-2 opacity-50"><use
href="#moon-stars-fill"></use></svg>
Dark
<svg class="bi ms-auto d-none"><use
href="#check2"></use></svg>
</button>
</li>
<li>
<button type="button" class="dropdown-item d-flex
align-items-center active" data-bs-theme-value="auto" aria-pressed="true">
<svg class="bi me-2 opacity-50"><use
href="#circle-half"></use></svg>
```
##########
landing-pages/site/assets/scss/_navbar.scss:
##########
@@ -99,16 +106,22 @@
font-size: 0.8125rem;
min-width: 7rem;
padding: 0.25rem 0;
- left: 0 !important;
- right: auto !important;
+ left: auto !important;
+ right: 0 !important;
Review Comment:
The dropdown menu positioning has been changed from `left: 0` to `right: 0`.
While this may be intentional for improved alignment, verify this doesn't cause
overflow issues on smaller screens or when the navbar is near the right edge of
the viewport. The transform adjustment (`transform: translateY(-2px)`) also
suggests positioning tweaks that should be tested across different screen sizes.
```suggestion
@media (max-width: $tablet) {
left: 0 !important;
right: auto !important;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
