Check every SVG in gitlab-svgs for a viewBox
What does this MR do?
The svgs:build-and-check job runs packages/gitlab-svgs/build_scripts/svg-lint.sh, which fails the pipeline for an SVG without a viewBox by running grep over **/*.svg. Bash does not enable globstar unless a script asks for it, so **/*.svg behaves like */*.svg and reaches one directory level only: sprite_icons/*.svg, illustrations/*.svg and the top level of dist/, file_icons/ and file_icons_unused/. The check never saw the SVGs in the illustration subdirectories or their optimized copies in dist/illustrations/, and 47 of those source files have no viewBox.
!6444 (merged) adds a step to the illustration export guide that asks for exactly this, a viewBox that matches the width and height, and tells contributors to export into illustrations/ or one of its subdirectories, which is where the check was not looking.
This MR enforces the rule the script already states, and brings the files it was missing into line with it:
- The check now reads every SVG under
packages/gitlab-svgsat any depth, leaving outnode_modules, with the samegreppattern and the same messages. After a build it reads 2,501 files instead of 1,343, and every file it read before is still among them. - The 47 files get
viewBox="0 0 W H", taken from each file's ownwidthandheight. Nothing else in them changes: each diff is that one attribute. - A patch changeset for
@gitlab/svgs.
This is not a fix for a break someone reported. At its own size, viewBox="0 0 W H" maps the drawing one to one onto the box it already had, so nothing moves. What changes is an inline <svg> sized with CSS: without a viewBox the drawing stays at its own size from the top left corner and a smaller box crops it, and with one it scales to fit. As an <img>, Chromium, Firefox and WebKit already scaled the three files I tried without one, so <img> use looks the same.
The built package changes in exactly those 47 optimized files and in the size fields of illustrations_individual.json and logos.json. dist/illustrations.svg is byte for byte the same, because svg-sprite already gave the six sprite entries among the 47 a viewBox of their own size.
The 47 files
Under packages/gitlab-svgs/illustrations/:
empty-state/:empty-agent-sessions-md,empty-ai-catalog-md,empty-artifacts-md,empty-artifacts-smerror/:error-404-lg,error-429-lglogos/:aws,bash,clojure,composer,c_plus_plus,crystal,dart,django,docker,elixir,fastlane,flutter,grails,hexo,jekyll,julia,kubernetes,laravel,mono,netlify,node_js,npm,openshift,packer,php,python,ruby,rust,scala,terraform,typo3third-party-logos/:chorus,ci_cd-template-logos/django,civo,gatsby,hotjar,integrations-logos/github,knowbe4,linux,siemens,wish
How I tested it
All in node:22.23.3-trixie, the job's NODE_IMAGE, running the job's own commands from a clean checkout: source scripts/utils.sh, cd packages/gitlab-svgs, retry yarn install --frozen-lockfile, bash ./build_scripts/build-svgs.sh, bash ./build_scripts/svg-lint.sh.
- On
main(be7677381), the current script passes. The new script fails and lists 94 files: the 47 sources and their 47 optimized copies indist/illustrations/. - On this branch the new script passes, which also covers every file under
dist/andfile_icons_unused/at any depth. - With
main's copy oferror/error-404-lg.svgput back and the package rebuilt, it fails again and listsdist/illustrations/error/error-404-lg.svgandillustrations/error/error-404-lg.svg. With the fix restored it passes. - In throwaway directories, the new check passes with no SVG at all (
xargs -rthen runs nogrep), fails on a file nested two levels down, names files whose names start with a dash or contain spaces, and ignores a file undernode_modules. shellcheckreports nothing on the new script. Onmainit reports SC2035 twice, for the two**/*.svgglobs.- Every changed SVG holds exactly one
viewBox, equal to0 0and its ownwidthandheight, and removing that attribute gives backmain's file byte for byte. - In headless Chrome 154, each file as an
<img>at its own size, before and after: 44 of the 47 render pixel for pixel the same. The other three have fractional sizes (kubernetes722.846 by 701.966,civo711.364 by 238.193,linux289.333 by 341.333) and differ in edge anti-aliasing on 310, 378 and 66 pixels, at most 0.22% of the image. - In Playwright 1.63.0's Chromium, Firefox and WebKit,
empty-artifacts-md,error-404-lgandlogos/rubyas<img>elements render pixel for pixel the same before and after: at their own size, at 72 by 72, and at half their height with no width, which is howGlEmptyStatesizes ansvg-path(itssvgHeightdefaults to 144, anderror-404-lgis 288 high). Inlined and sized to 72 by 72 with CSS, they differ on 772 to 3,331 of the 5,184 pixels: before this change each shows only the top left corner of its drawing at full size, and after it each renders exactly like the 72 by 72<img>.
Screenshots or screen recordings
None. As an <img>, each file I tried looks the same before and after in Chromium, Firefox and WebKit, at its own size and scaled down, and every reference to these 47 files I found in gitlab-org/gitlab loads them as an image (image_tag, image_path or a webpack URL import). The difference shows only when one of them is inlined and sized with CSS, as described above.
Integrations
No file is added, renamed or removed, so no project needs a migration.
- GitLab: mr_url (Run the
ui:create_integration_merge_requestCI job, which automatically creates the merge request and updates this item for you.) - CustomersDot: mr_url
- Duo UI: mr_url
- Status Page: mr_url
- Docs: mr_url
- Switchboard: mr_url
Does this MR meet the acceptance criteria?
This checklist encourages the authors, reviewers, and maintainers of merge requests (MRs) to confirm changes were analyzed for conformity with the project's guidelines, security and accessibility.
Toggle the acceptance checklist
Conformity
- The “What does this MR do?” section in the MR description is filled out, explaining the reasons for and scope of the proposed changes, per “Say why not just what”.
- For example, if the MR is focused on usage guidelines, addressing accessibility challenges could be added in a separate MR.
- Relevant label(s) are applied to the MR.
- The MR is added to a milestone.
- Added the
~"component:*"label(s) if applicable. - A changeset is added when this MR will do a patch, minor, or major update. More information can be found in
doc/changesets.md.
Components
- GitLab UI's contributing guidelines.
- If the MR changes a component's API, integration MR(s) have been opened (see integrations above).
Documentation
- If creating a new component page from scratch, it follows the page template structure.
- Content follows the GitLab Documentation Style Guide when appropriate.
- Related pages are cross-linked, where helpful. Component pages have related components and patterns defined in their Markdown front matter.
- If embedding a Figma file, it follows the Figma embed guide.
- Review requested from any GitLab designer or directly from a maintainer or trainee maintainer.
- Maintainer review follows the Pajamas UX maintainer review checklist.
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- Label as security and @ mention
@gitlab-com/gl-security/appsec - Security reports checked/validated by a reviewer from the AppSec team
Accessibility
If this MR adds or modifies a component, take a few moments to review the following:
- All actions and functionality can be done with a keyboard.
- Links, buttons, and controls have a visible focus state.
- All content is presented in text or with a text equivalent. For example, alt text for SVG, or
aria-labelfor icons that have meaning or perform actions. - Changes in a component’s state are announced by a screen reader. For example, changing
aria-expanded="false"toaria-expanded="true"when an accordion is expanded. - Color combinations have sufficient contrast.