fix : add background styling for dark logos in branding documentation - #247
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe branding page wraps dark-background logo images in Merge Risk: ⚪ Minimal · up to This localized documentation styling change adds dark backgrounds behind branding logos and introduces no actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ekagrashharma
changed the title
| | [svg](https://github.com/openjs-foundation/artwork/blob/main/projects/webpack/webpack-logo-horizontal-color.svg) \| [png](https://github.com/openjs-foundation/artwork/blob/main/projects/webpack/webpack-logo-horizontal-color.png) | [svg](https://github.com/openjs-foundation/artwork/blob/main/projects/webpack/webpack-logo-horizontal-color-dark_background.svg) \| [png](https://github.com/openjs-foundation/artwork/blob/main/projects/webpack/webpack-logo-horizontal-color-dark_background.png) | | ||
| | Light backgrounds | Dark backgrounds | | ||
| | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | | ||
| |  | <span style="background-color: #0d121c; padding: 20px; display: inline-block;"></span> | |
There was a problem hiding this comment.
Can we use a -- variable, we have several theme variables defined
There was a problem hiding this comment.
Okay, I understand. Should I use one of the existing theme variables for this, or would you prefer me to define a new variable for the color?
There was a problem hiding this comment.
Hi @Ekagrashharma! I encourage you to read webpack's AI policy.
There was a problem hiding this comment.
Sure, I’ll go through the AI policy and make sure I’m aligned with that .
Just wanted to be transparent with you—I’m not too much confident with my English, so I sometimes use AI to help me phrase my messages better. :)
There was a problem hiding this comment.
Hey, @avivkeller, can you help me with what to do next
There was a problem hiding this comment.
Hey, @TusharThakur04
I tried to add the variable instead of the hex code, but Tailwind is not loading in the MDX file. I read the CSS and style loader file, but I am unclear on how to tackle this.
Can you give me some references where I can look into this ?
There was a problem hiding this comment.
@avivkeller does mdx parser at doc-kit take care of variable names?
There was a problem hiding this comment.
The variable name is plain CSS, so it's not parsed/tailwind-ed, and is handled by the browser's CSS loading directly
There was a problem hiding this comment.
@Ekagrashharma If you push your changes that aren't working, I'm sure I can help identify a root cause :-)
There was a problem hiding this comment.
@Ekagrashharma If you push your changes that aren't working, I'm sure I can help identify a root cause :-)
OK, I'll do that, and I also added 2 test divs so you can see that Tailwind is not loading, but the inline CSS is working.
- the theme.css is declaring the @theme variables
github-actions
Bot
added
the
dependencies
|
@avivkeller Just wondering ,should we make these logos consistent or does that conflict with the branding guidelines? Just checking . |
Define making them consistent? |
|
size i guess |
I agree on that, the logo sizes are wildly different. |
Yeah, I agree with that. Should I add a commit to fix this or make a new issue ? |
WDYT? it this better ? @TusharThakur04 |
yes i meant the sizing of the logos |
Just to confirm, are you saying that the logos have different dimensions? |
the ask is that the different logos use similar proportional sizes. Some logos are huge. |
|
Yes, exactly. they have different dimensions, and some appear much larger than others. |
Co-authored-by: Aviv Keller <me@aviv.sh>
c2a0698 to
718cfb4
Compare
github-actions
Bot
removed
the
dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b93abd28-07f8-4fc0-830f-c5920bffabe2
📒 Files selected for processing (2)
pages/about/branding.mdstyles/index.css
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
webpack/webpack(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/about/branding.md
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| .darkBg { | ||
| background-color: var(--color-neutral-950); | ||
| display: inline-block; | ||
| padding: 20px; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect image rules that keep the logos within their 50% table cells.
rg -n -C 4 '\.darkBg|\.halfWidthTable|img|max-width|max-height|overflow' \
styles pages/about/branding.mdRepository: webpack/webpack-doc-kit
Length of output: 855
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- branding references and image syntax ---'
rg -n -C 8 'darkBg|halfWidthTable|!\[|<img|image|logo' pages/about/branding.md pages styles || true
printf '%s\n' '--- stylesheet structure and image rules ---'
fd -t f -e css -e scss -e md . styles pages | sort
rg -n -C 3 '(^|[,{[:space:]])img([[:space:]:,{]|$)|max-width|max-height|width:|height:|overflow' styles pages || true
printf '%s\n' '--- relevant file sizes ---'
wc -l styles/index.css pages/about/branding.mdRepository: webpack/webpack-doc-kit
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
css = Path("styles/index.css").read_text()
md = Path("pages/about/branding.md").read_text()
print("--- CSS declarations ---")
for selector, body in re.findall(r"([^{}]+)\{([^{}]*)\}", css):
if any(term in selector for term in ("darkBg", "halfWidthTable", "img")):
print(selector.strip(), "=>", " ".join(body.split()))
print("--- branding image lines and wrappers ---")
for number, line in enumerate(md.splitlines(), 1):
if any(term in line for term in ("darkBg", "halfWidthTable", "![", "<img")):
print(f"{number}: {line}")
print("--- intrinsic image dimensions in repository ---")
for path in Path(".").rglob("*"):
if path.is_file() and path.suffix.lower() in {".png", ".jpg", ".jpeg", ".svg", ".webp"}:
print(path)
PYRepository: webpack/webpack-doc-kit
Length of output: 3313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all stylesheet files ---'
fd -t f \( -e css -o -e scss \) . | sort
printf '%s\n' '--- image and table rules in stylesheets ---'
while IFS= read -r file; do
printf '\n[%s]\n' "$file"
rg -n -C 3 '(^|[,{[:space:]])img([[:space:]:,{]|$)|max-width|max-height|width:|height:|overflow|table-layout|halfWidthTable|darkBg' "$file" || true
done < <(fd -t f \( -e css -o -e scss \) . | sort)
printf '%s\n' '--- layout/config references ---'
rg -n -C 3 'styles/index\.css|index\.css|markdown|remark|rehype|table' package.json *.js *.cjs *.mjs *.ts config* scripts plugins 2>/dev/null || trueRepository: webpack/webpack-doc-kit
Length of output: 303
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
from struct import unpack
urls = {
"horizontal-light": "https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color.png",
"horizontal-dark": "https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color-dark_background.png",
"stacked-light": "https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color.png",
"stacked-dark": "https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color-dark_background.png",
}
for name, url in urls.items():
with urlopen(url, timeout=15) as response:
data = response.read()
if data[:8] != b"\x89PNG\r\n\x1a\n":
raise RuntimeError(f"{name}: response is not PNG")
width, height = unpack(">II", data[16:24])
print(f"{name}: {width}x{height}, bytes={len(data)}")
PY
Length of output: 2412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all stylesheet files ---'
fd --type f --extension css --extension scss . | sort
printf '%s\n' '--- image and table rules in stylesheets ---'
while IFS= read -r file; do
printf '\n[%s]\n' "$file"
rg -n -C 3 '(^|[,{[:space:]])img([[:space:]:,{]|$)|max-width|max-height|width:|height:|overflow|table-layout|halfWidthTable|darkBg' "$file" || true
done < <(fd --type f --extension css --extension scss . | sort)
printf '%s\n' '--- layout/config references ---'
rg -n -C 3 'styles/index\.css|index\.css|markdown|remark|rehype|table' package.json '*.js' '*.cjs' '*.mjs' '*.ts' config* scripts plugins 2>/dev/null || trueRepository: webpack/webpack-doc-kit
Length of output: 1158
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
for url in \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color-dark_background.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color-dark_background.png'
do
printf '%s: ' "$url"
curl -ksSL --fail "$url" | python3 -c '
import sys, struct
data = sys.stdin.buffer.read()
if data[:8] != b"\x89PNG\r\n\x1a\n":
raise SystemExit("not a PNG")
print(f"{struct.unpack(\">II\", data[16:24])[0]}x{struct.unpack(\">II\", data[16:24])[1]}")
'
done
Length of output: 625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- global stylesheets ---'
for file in styles/index.css styles/theme.css; do
printf '\n[%s]\n' "$file"
cat -n "$file"
done
printf '%s\n' '--- image rules across stylesheets ---'
rg -n -C 4 '(^|[,{[:space:]])img([[:space:]:,{]|$)|max-width|max-height|overflow' \
styles components layouts || true
printf '%s\n' '--- branding table scope ---'
sed -n '18,38p' pages/about/branding.mdRepository: webpack/webpack-doc-kit
Length of output: 11940
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
for url in \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-horizontal-color-dark_background.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color.png' \
'https://raw.githubusercontent.com/openjs-foundation/artwork/main/projects/webpack/webpack-logo-stacked-color-dark_background.png'
do
printf '%s: ' "$url"
curl -ksSL --fail "$url" -o /tmp/logo.png
python3 - <<'PY'
import struct
data = open("/tmp/logo.png", "rb").read()
if data[:8] != b"\x89PNG\r\n\x1a\n":
raise SystemExit("not a PNG")
width, height = struct.unpack(">II", data[16:24])
print(f"{width}x{height}")
PY
done
Length of output: 658
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Tailwind version and preflight source ---'
rg -n '"tailwindcss"|tailwindcss@|preflight' package.json package-lock.json pnpm-lock.yaml yarn.lock node_modules/tailwindcss 2>/dev/null | head -80 || true
find node_modules/tailwindcss -maxdepth 3 -type f \( -iname '*preflight*' -o -iname '*.css' \) -print 2>/dev/null | head -40
printf '%s\n' '--- markdown renderer and article classes ---'
rg -n -C 5 'ReactMarkdown|remark|rehype|dangerouslySetInnerHTML|prose|article|mdx|markdown' \
components layouts plugins scripts package.json 2>/dev/null | head -240 || true
printf '%s\n' '--- exact branding wrapper scope ---'
python3 - <<'PY'
from pathlib import Path
text = Path("pages/about/branding.md").read_text()
print("halfWidthTable openings:", text.count('<div class="halfWidthTable">'))
print("halfWidthTable closings:", text.count("</div>"))
print("stacked table is inside wrapper:", text.index("### Stacked logo") < text.index("</div>"))
PYRepository: webpack/webpack-doc-kit
Length of output: 315
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- Tailwind package files ---'
find node_modules/tailwindcss -maxdepth 3 -type f \( -iname '*preflight*' -o -iname '*.css' \) -print 2>/dev/null | head -40 || true
printf '%s\n' '--- Tailwind preflight references ---'
rg -n -C 4 'max-width: 100%|height: auto|img, video|preflight' \
node_modules/tailwindcss package-lock.json 2>/dev/null | head -120 || true
printf '%s\n' '--- markdown renderer and article classes ---'
rg -n -C 5 'ReactMarkdown|remark|rehype|dangerouslySetInnerHTML|prose|article|mdx|markdown' \
components layouts plugins scripts package.json 2>/dev/null | head -240 || true
printf '%s\n' '--- exact branding wrapper scope ---'
python3 - <<'PY'
from pathlib import Path
text = Path("pages/about/branding.md").read_text()
open_at = text.index('<div class="halfWidthTable">')
close_at = text.index("</div>", open_at)
stacked_at = text.index("### Stacked logo")
print("wrapper opening:", open_at)
print("wrapper closing:", close_at)
print("stacked heading:", stacked_at)
print("stacked table inside wrapper:", open_at < stacked_at < close_at)
PYRepository: webpack/webpack-doc-kit
Length of output: 12385
🌐 Web query:
Tailwind CSS v4.3.3 preflight img video max-width 100% height auto
💡 Result:
In Tailwind CSS v4.3.3, Preflight includes a base style rule that constrains images and videos to prevent them from overflowing their containers [1][2]: img, video { max-width: 100%; height: auto; } This rule ensures that images and videos are responsive by default, maintaining their intrinsic aspect ratio [1][2]. If you need to override this behavior, you can use the max-w-none utility class [1][2]. For cases where you must remove the height: auto rule entirely, common approaches include [3][4]: 1. Disabling Preflight: You can disable Preflight globally if you prefer to manage your own base resets [3][4]. 2. Using Utility Overrides: While it is difficult to revert height: auto to the default browser behavior using simple CSS values, you may need to apply specific heights or use other layout strategies if the default behavior conflicts with your design [3][4]. 3. Custom PostCSS Plugins: Some developers use plugins to strip specific rules from the generated CSS after Tailwind runs [4]. Please note that Tailwind v4.3.3 introduced a fix regarding Preflight and iframes to prevent it from overriding Firefox's native focus-visible outline styles, but the core image and video constraint rules remain unchanged [5][6][7].
Citations:
- 1: https://tailwindcss.com/docs/preflight
- 2: https://github.com/tailwindlabs/tailwindcss.com/blob/main/src/docs/preflight.mdx
- 3: How to remove height: auto tailwindlabs/tailwindcss#18098
- 4: Dont set height to auto if an img tag has an height tailwindlabs/tailwindcss#7742
- 5: https://github.com/tailwindlabs/tailwindcss/releases/tag/v4.3.3
- 6: tailwindlabs/tailwindcss@v4.3.2...v4.3.3
- 7: https://newreleases.io/project/github/tailwindlabs/tailwindcss/release/v4.3.3
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused change ---'
git diff --unified=12 -- styles/index.css pages/about/branding.md
printf '%s\n' '--- available browser/layout tools ---'
for command in chromium chromium-browser google-chrome firefox playwright; do
if command -v "$command" >/dev/null 2>&1; then
printf '%s: %s\n' "$command" "$(command -v "$command")"
fi
done
printf '%s\n' '--- relevant package metadata ---'
sed -n '1,80p' package.jsonRepository: webpack/webpack-doc-kit
Length of output: 2226
🌐 Web query:
CSS inline-block wrapper padding child img max-width 100% intrinsic width overflow table cell
💡 Result:
The layout issues you are encountering when nesting an image with max-width: 100% inside an inline-block wrapper—especially within a table cell—stem from cyclic dependencies in how browsers calculate intrinsic widths [1][2][3]. When a container (like an inline-block or a table cell) has an auto width, its size is determined by its content [2][4]. If a child image within that container is set to max-width: 100%, the browser faces a circular dependency: it cannot determine the width of the container without the image, but it cannot determine the width of the image without knowing the width of the container [1][2][3]. Key causes and solutions for these behaviors are detailed below: Circular Dependency in Intrinsic Sizing When the wrapper is inline-block and the child image is set to max-width: 100%, the browser may default to the image's intrinsic (natural) pixel size rather than scaling it to the available space [1][5]. This often causes the wrapper (and consequently the table cell) to expand to the full natural width of the image [1][6][7]. Baseline Gaps Images are inline elements by default [8][6]. Even if you set display: block on an image, it often leaves a small gap at the bottom of a container because it aligns to the baseline of the text, reserving space for typographic descenders [9][10]. To eliminate this, ensure the image is set to display: block or vertical-align: top/bottom [11][9][6]. Fixing Table Cell Overflow If images cause a table cell or its wrapper to exceed expected bounds, consider these strategies: - Use the min function: If you need responsiveness while preventing expansion beyond natural size, use width: min(100%, px) [1]. This breaks the circular dependency by bounding the image's contribution to the table's auto-layout calculation [1]. - Enforce layout: Setting table-layout: fixed on the table can prevent cells from expanding based on image content, forcing them to respect defined column widths [8][12]. - Force Block Behavior: Always set display: block on images inside wrappers to remove inline-related spacing issues [11][6]. - Use min-width: 100% on the wrapper: If using an inline-block wrapper inside an overflow container, adding min-width: 100% ensures the wrapper correctly fills the available scrollable space rather than shrinking to fit the image [13]. If you are using percentage-based padding on the wrapper or child, the browser may ignore the padding during the initial intrinsic width calculation to avoid an infinite loop [3]. Using fixed units (e.g., px, em) for padding often resolves these layout calculation failures [3].
Citations:
- 1: Fix image overflow in table cells using CSS min() outline/outline#12902
- 2: https://stackoverflow.com/questions/63001886/how-do-browsers-calculate-width-when-child-depends-on-parent-and-parents-depen
- 3: https://stackoverflow.com/questions/58289173/display-inline-block-not-growing-horizontally-with-child-having-padding-in-per-c
- 4: https://css-tricks.com/exploring-the-complexities-of-width-and-height-in-css/
- 5: https://stackoverflow.com/questions/45829704/why-an-image-with-100-width-in-inline-block-behaving-as-responsive-image
- 6: https://stackoverflow.com/questions/5791033/image-expands-the-table-cell
- 7: https://stackoverflow.com/questions/3201230/how-to-shrink-a-div-around-a-scaled-img
- 8: https://stackoverflow.com/questions/38255321/ie10-max-width-fails-on-image-inside-fixed-layout-table-columns
- 9: https://stackoverflow.com/questions/78930153/vertical-gaps-between-table-cells-and-their-contents
- 10: https://stackoverflow.com/questions/48229825/can-you-explain-extra-padding-on-the-3rd-table-when-content-has-overflow-hidden
- 11: https://meyerweb.com/eric/articles/devedge/img-table-gaps/
- 12: https://stackoverflow.com/questions/23515798/table-cell-unnecessarily-overflows-wrapper-in-ff-and-ie
- 13: https://www.bennadel.com/blog/3880-using-inline-block-to-provide-consistent-padding-and-element-width-inside-an-overflow-container-in-css.htm
Constrain .darkBg and wrap the stacked logo table.
Tailwind Preflight constrains the direct <img>, but .darkBg can retain the nested image’s intrinsic width. Its 20px padding can then make the dark cell overflow its fixed 50% column on narrow screens. Add scoped max-width: 100% and box-sizing: border-box rules to .darkBg.
The stacked logo table is outside .halfWidthTable, so it does not receive the fixed-layout or 50% column rules. Move the closing </div> after that table if both tables require equal columns.
✅ Addressed in commit 002dc7c
There was a problem hiding this comment.
@Ekagrashharma what about the third table 👀
working on it
There was a problem hiding this comment.
which one?
The one under them that is the sticker ;)
There was a problem hiding this comment.
Hey @Ekagra! I was checking the recent commit. It looks like you added some custom CSS to fix the width of the third table. Actually, the reviewer just wanted us to move the closing </div> of .halfWidthTable down in the Markdown file so it wraps the third table as well. 👀
Thank you; it means a lot :) |
final changes. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
styles/index.css (1)
24-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent
.darkBgfrom overflowing narrow table cells.At Line 24,
.darkBgusesdisplay: inline-blockandpadding: 20pxwithout a width constraint. The nested logo can retain its intrinsic width, causing the wrapper to exceed the fixed 50% column on narrow screens. Addbox-sizing: border-boxandmax-width: 100%to.darkBg, and constrain its nested image.Proposed fix
.darkBg { background: `#0d121c`; + box-sizing: border-box; display: inline-block; + max-width: 100%; padding: 20px; } + +.darkBg img { + max-width: 100%; +}
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9b0e8c4-42d3-42ff-9b5c-e63af8f559ae
📒 Files selected for processing (2)
pages/about/branding.mdstyles/index.css
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
webpack/webpack(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/about/branding.md
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
Thanks to all the maintainers for your support and guidance in getting this issue merged. I really appreciate it! :) Could you also guide me on how I can start tackling more challenging issues? |
Summary
This pull request updates the pages/about/branding.md file to improve the presentation of the webpack logos on dark backgrounds. The main change is the addition of a dark background behind the logo images to ensure better visibility and consistency.
fixes : #243
Dark theme
Light theme
Summary by CodeRabbit