From cdd68fee7fc7ab48959fcefabdb81dec4747cce7 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Wed, 26 May 2021 15:55:28 -0400 Subject: [PATCH] Make for-the-badge letter spacing more predictable, and rewrite layout logic (#5754) * Rewrite for-the-badge renderer * Update snapshots * Remove pixel grid alignment in for-the-badge * abstract XML stringification to XmlElement class Co-authored-by: chris48s Co-authored-by: chris48s --- __snapshots__/make-badge.spec.js | 129 ++++++------ badge-maker/lib/badge-renderers.js | 309 +++++++++++++++++++---------- badge-maker/lib/make-badge.js | 5 +- badge-maker/lib/xml.js | 76 +++++++ badge-maker/lib/xml.spec.js | 50 +++++ 5 files changed, 394 insertions(+), 175 deletions(-) create mode 100644 badge-maker/lib/xml.js create mode 100644 badge-maker/lib/xml.spec.js diff --git a/__snapshots__/make-badge.spec.js b/__snapshots__/make-badge.spec.js index 877d17bb5e..1756aabdbd 100644 --- a/__snapshots__/make-badge.spec.js +++ b/__snapshots__/make-badge.spec.js @@ -969,15 +969,15 @@ exports['The badge generator "for-the-badge" template badge generation should ma CACTUS: GROWN - - + + - + CACTUS GROWN @@ -1008,15 +1008,15 @@ exports['The badge generator "for-the-badge" template badge generation should ma CACTUS: GROWN - - + + - + CACTUS GROWN @@ -1054,15 +1054,14 @@ exports['The badge generator "for-the-badge" template badge generation should ma GROWN - - + GROWN @@ -1090,15 +1089,14 @@ exports['The badge generator "for-the-badge" template badge generation should ma GROWN - - + GROWN @@ -1133,7 +1131,7 @@ exports['The badge generator "for-the-badge" template badge generation should ma GROWN - + - GROWN @@ -1183,12 +1174,12 @@ exports['The badge generator "for-the-badge" template badge generation should ma - - + + - - + + CACTUS - + GROWN @@ -1911,15 +1908,15 @@ exports['The badge generator text colors should use black text when the message CACTUS: GROWN - - + + - + CACTUS GROWN diff --git a/badge-maker/lib/badge-renderers.js b/badge-maker/lib/badge-renderers.js index 66b51b47db..6aba74624a 100644 --- a/badge-maker/lib/badge-renderers.js +++ b/badge-maker/lib/badge-renderers.js @@ -2,8 +2,14 @@ const anafanafo = require('anafanafo') const { brightness } = require('./color') +const { XmlElement, escapeXml } = require('./xml') -const fontFamily = 'font-family="Verdana,Geneva,DejaVu Sans,sans-serif"' +// https://github.com/badges/shields/pull/1132 +const FONT_SCALE_UP_FACTOR = 10 +const FONT_SCALE_DOWN_VALUE = 'scale(.1)' + +const FONT_FAMILY = 'Verdana,Geneva,DejaVu Sans,sans-serif' +const fontFamily = `font-family="${FONT_FAMILY}"` const socialFontFamily = 'font-family="Helvetica Neue,Helvetica,Arial,sans-serif"' const brightnessThreshold = 0.69 @@ -20,19 +26,6 @@ function colorsForBackground(color) { } } -function escapeXml(s) { - if (s === undefined || typeof s !== 'string') { - return undefined - } else { - return s - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, ''') - } -} - function roundUpToOdd(val) { return val % 2 === 0 ? val + 1 : val } @@ -576,126 +569,232 @@ function forTheBadge({ links, logo, logoWidth, - logoPadding, color = '#4c1', labelColor, }) { - // For the Badge is styled in all caps. Convert to caps here so widths can - // be measured using the correct characters. + const FONT_SIZE = 10 + const BADGE_HEIGHT = 28 + const LOGO_HEIGHT = 14 + const TEXT_MARGIN = 12 + const LOGO_MARGIN = 9 + const LOGO_TEXT_GUTTER = 6 + const LETTER_SPACING = 1.25 + + // Prepare content. For the Badge is styled in all caps. It's important to to + // convert to uppercase first so the widths can be measured using the correct + // symbols. label = label.toUpperCase() message = message.toUpperCase() - let labelWidth = preferredWidthOf(label, { font: '10px Verdana' }) || 0 - let messageWidth = - preferredWidthOf(message, { font: 'bold 10px Verdana' }) || 0 - const height = 28 - const hasLabel = label.length || labelColor - if (labelColor == null) { - labelColor = '#555' - } - const horizPadding = 9 - const { hasLogo, totalLogoWidth, renderedLogo } = renderLogo({ - logo, - badgeHeight: height, - horizPadding, - logoWidth, - logoPadding, - }) - - labelWidth += 10 + totalLogoWidth - if (label.length) { - // Add 10 px of padding, plus approximately 1 px of letter spacing per - // character. - labelWidth += 10 + 2 * label.length - } else if (hasLogo) { - if (hasLabel) { - labelWidth += 7 - } else { - labelWidth -= 7 - } - } else { - labelWidth -= 11 - } - - // Add 20 px of padding, plus approximately 1.5 px of letter spacing per - // character. - messageWidth += 20 + 1.5 * message.length - const leftWidth = hasLogo && !hasLabel ? 0 : labelWidth - const rightWidth = - hasLogo && !hasLabel ? messageWidth + labelWidth : messageWidth - - labelColor = hasLabel || hasLogo ? labelColor : color - - color = escapeXml(color) - labelColor = escapeXml(labelColor) - - let [leftLink, rightLink] = links - leftLink = escapeXml(leftLink) - rightLink = escapeXml(rightLink) + const [leftLink, rightLink] = links const { hasLeftLink, hasRightLink } = hasLinks({ links }) - const accessibleText = createAccessibleText({ label, message }) + const outLabelColor = labelColor || '#555' - function renderLabelText() { - const { textColor } = colorsForBackground(labelColor) - const labelTextX = ((labelWidth + totalLogoWidth) / 2) * 10 - const labelTextLength = (labelWidth - (24 + totalLogoWidth)) * 10 - const escapedLabel = escapeXml(label) + // Compute text width. + // TODO: This really should count the symbols rather than just using `.length`. + // https://mathiasbynens.be/notes/javascript-unicode + // This is not using `preferredWidthOf()` as it tends to produce larger + // inconsistencies in the letter spacing. The badges look fine, however if you + // replace `textLength` with `letterSpacing` in the rendered SVG, you can see + // the discrepancy. Ideally, swapping out `textLength` for `letterSpacing` + // should not affect the appearance. + const labelTextWidth = label.length + ? (anafanafo(label, { font: `${FONT_SIZE}px Verdana` }) | 0) + + LETTER_SPACING * label.length + : 0 + const messageTextWidth = message.length + ? (anafanafo(message, { font: `bold ${FONT_SIZE}px Verdana` }) | 0) + + LETTER_SPACING * message.length + : 0 - const text = `${escapedLabel}` + // Compute horizontal layout. + // If a `labelColor` is set, the logo is always set against it, even when + // there is no label. When `needsLabelRect` is true, render a label rect and a + // message rect; when false, only a message rect. + const hasLabel = Boolean(label.length) + const needsLabelRect = hasLabel || (logo && labelColor) + let logoMinX, labelTextMinX + if (logo) { + logoMinX = LOGO_MARGIN + labelTextMinX = logoMinX + logoWidth + LOGO_TEXT_GUTTER + } else { + labelTextMinX = TEXT_MARGIN + } + let labelRectWidth, messageTextMinX, messageRectWidth + if (needsLabelRect) { + if (hasLabel) { + labelRectWidth = labelTextMinX + labelTextWidth + TEXT_MARGIN + } else { + labelRectWidth = 2 * LOGO_MARGIN + logoWidth + } + messageTextMinX = labelRectWidth + TEXT_MARGIN + messageRectWidth = 2 * TEXT_MARGIN + messageTextWidth + } else { + if (logo) { + messageTextMinX = TEXT_MARGIN + logoWidth + LOGO_TEXT_GUTTER + messageRectWidth = + 2 * TEXT_MARGIN + logoWidth + LOGO_TEXT_GUTTER + messageTextWidth + } else { + messageTextMinX = TEXT_MARGIN + messageRectWidth = 2 * TEXT_MARGIN + messageTextWidth + } + } + + const logoElement = new XmlElement({ + name: 'image', + attrs: { + x: logoMinX, + y: 0.5 * (BADGE_HEIGHT - LOGO_HEIGHT), + width: logoWidth, + height: LOGO_HEIGHT, + 'xlink:href': logo, + }, + }) + + function getLabelElement() { + const { textColor } = colorsForBackground(outLabelColor) + const midX = labelTextMinX + 0.5 * labelTextWidth + const text = new XmlElement({ + name: 'text', + content: [label], + attrs: { + transform: FONT_SCALE_DOWN_VALUE, + x: FONT_SCALE_UP_FACTOR * midX, + y: 175, + textLength: FONT_SCALE_UP_FACTOR * labelTextWidth, + fill: textColor, + }, + }) if (hasLeftLink && !shouldWrapBodyWithLink({ links })) { - return ` - - - ${text} - - ` + const rect = new XmlElement({ + name: 'rect', + attrs: { + width: labelRectWidth, + height: BADGE_HEIGHT, + fill: 'rgba(0,0,0,0)', + }, + }) + return new XmlElement({ + name: 'a', + content: [rect, text], + attrs: { + target: '_blank', + 'xlink:href': leftLink, + }, + }) } else { return text } } - function renderMessageText() { + function getMessageElement() { const { textColor } = colorsForBackground(color) - - const text = ` - ${escapeXml(message)}` + const midX = messageTextMinX + 0.5 * messageTextWidth + const text = new XmlElement({ + name: 'text', + content: [message], + attrs: { + transform: FONT_SCALE_DOWN_VALUE, + x: FONT_SCALE_UP_FACTOR * midX, + y: 175, + textLength: FONT_SCALE_UP_FACTOR * messageTextWidth, + fill: textColor, + 'font-weight': 'bold', + }, + }) if (hasRightLink) { - return ` - - - ${text} - - ` + const rect = new XmlElement({ + name: 'rect', + attrs: { + width: messageRectWidth, + height: BADGE_HEIGHT, + x: labelRectWidth || 0, + fill: 'rgba(0,0,0,0)', + }, + }) + return new XmlElement({ + name: 'a', + content: [rect, text], + attrs: { + target: '_blank', + 'xlink:href': rightLink, + }, + }) } else { return text } } + let backgroundContent + if (needsLabelRect) { + backgroundContent = [ + new XmlElement({ + name: 'rect', + attrs: { + width: labelRectWidth, + height: BADGE_HEIGHT, + fill: outLabelColor, + }, + }), + new XmlElement({ + name: 'rect', + attrs: { + x: labelRectWidth, + width: messageRectWidth, + height: BADGE_HEIGHT, + fill: color, + }, + }), + ] + } else { + backgroundContent = [ + new XmlElement({ + name: 'rect', + attrs: { + width: messageRectWidth, + height: BADGE_HEIGHT, + fill: color, + }, + }), + ] + } + + const backgroundGroup = new XmlElement({ + name: 'g', + content: backgroundContent, + attrs: { + 'shape-rendering': 'crispEdges', + }, + }) + const foregroundGroup = new XmlElement({ + name: 'g', + content: [ + logo ? logoElement : '', + hasLabel ? getLabelElement() : '', + getMessageElement(), + ], + attrs: { + fill: '#fff', + 'text-anchor': 'middle', + 'font-family': FONT_FAMILY, + 'text-rendering': 'geometricPrecision', + 'font-size': FONT_SCALE_UP_FACTOR * FONT_SIZE, + }, + }) + + // Render. return renderBadge( { links, - leftWidth, - rightWidth, - accessibleText, - height, + leftWidth: labelRectWidth || 0, + rightWidth: messageRectWidth, + accessibleText: createAccessibleText({ label, message }), + height: BADGE_HEIGHT, }, - ` - - - - - - ${renderedLogo} - ${hasLabel ? renderLabelText() : ''} - ${renderMessageText()} - ` + [backgroundGroup.render(), foregroundGroup.render()].join('') ) } diff --git a/badge-maker/lib/make-badge.js b/badge-maker/lib/make-badge.js index 03fab6312b..495dd4ecbe 100644 --- a/badge-maker/lib/make-badge.js +++ b/badge-maker/lib/make-badge.js @@ -2,10 +2,7 @@ const { normalizeColor, toSvgColor } = require('./color') const badgeRenderers = require('./badge-renderers') - -function stripXmlWhitespace(xml) { - return xml.replace(/>\s+/g, '>').replace(/<\s+/g, '<').trim() -} +const { stripXmlWhitespace } = require('./xml') /* note: makeBadge() is fairly thinly wrapped so if we are making changes here diff --git a/badge-maker/lib/xml.js b/badge-maker/lib/xml.js new file mode 100644 index 0000000000..1d30fa501e --- /dev/null +++ b/badge-maker/lib/xml.js @@ -0,0 +1,76 @@ +/** + * @module + */ + +'use strict' + +function stripXmlWhitespace(xml) { + return xml.replace(/>\s+/g, '>').replace(/<\s+/g, '<').trim() +} + +function escapeXml(s) { + if (typeof s === 'number') { + return s + } else if (s === undefined || typeof s !== 'string') { + return undefined + } else { + return s + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') + } +} + +/** + * Representation of an XML element + */ +class XmlElement { + /** + * Xml Element Constructor + * + * @param {object} attrs Refer to individual attrs + * @param {string} attrs.name + * Name of the XML tag + * @param {Array.} [attrs.content=[]] + * Array of objects to render inside the tag. content may contain a mix of + * string and XmlElement objects. If content is `[]` or ommitted the + * element will be rendered as a self-closing element. + * @param {object} [attrs.attrs={}] + * Object representing the tag's attributes as name/value pairs + */ + constructor({ name, content = [], attrs = {} }) { + this.name = name + this.content = content + this.attrs = attrs + } + + /** + * Render the XML element to a string, applying appropriate escaping + * + * @returns {string} String representation of the XML element + */ + render() { + const attrsStr = Object.entries(this.attrs) + .map(([k, v]) => ` ${k}="${escapeXml(v)}"`) + .join('') + if (this.content.length > 0) { + const content = this.content + .map(function (el) { + if (el instanceof XmlElement) { + return el.render() + } else { + return escapeXml(el) + } + }) + .join(' ') + return stripXmlWhitespace( + `<${this.name}${attrsStr}>${content}` + ) + } + return stripXmlWhitespace(`<${this.name}${attrsStr}/>`) + } +} + +module.exports = { escapeXml, stripXmlWhitespace, XmlElement } diff --git a/badge-maker/lib/xml.spec.js b/badge-maker/lib/xml.spec.js new file mode 100644 index 0000000000..78ea84bada --- /dev/null +++ b/badge-maker/lib/xml.spec.js @@ -0,0 +1,50 @@ +'use strict' + +const { test, given } = require('sazerac') +const { XmlElement } = require('./xml') + +function testRender(params) { + return new XmlElement(params).render() +} + +describe('XmlElement class', function () { + test(testRender, () => { + given({ name: 'tag' }).expect('') + + given({ name: 'tag', content: ['text'] }).expect('text') + + given({ + name: 'tag', + content: ['not xml>>>', 'text', new XmlElement({ name: 'xml' })], + }).expect('not xml>>> text ') + + given({ + name: 'nested1', + content: [ + new XmlElement({ + name: 'nested2', + content: [new XmlElement({ name: 'nested3' })], + }), + ], + }).expect('') + + given({ + name: 'tag', + attrs: { + int: 47, + text: 'text', + escape: '', + }, + }).expect('') + + given({ + name: 'tag', + content: ['text'], + attrs: { + int: 47, + text: 'text', + escape: '', + }, + }).expect('text') + }) +})