Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove logoPosition #10284

Merged
merged 2 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions badge-maker/lib/make-badge.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ module.exports = function makeBadge({
color,
labelColor,
logo,
logoPosition,
logoSize,
logoWidth,
links = ['', ''],
Expand Down Expand Up @@ -55,7 +54,6 @@ module.exports = function makeBadge({
message,
links,
logo,
logoPosition,
logoWidth,
logoSize,
logoPadding: logo && label.length ? 3 : 0,
Expand Down
1 change: 0 additions & 1 deletion core/base-service/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ const serviceDataSchema = Joi.object({
logoSvg: Joi.string(),
logoColor: optionalStringWhenNamedLogoPresent,
logoWidth: optionalNumberWhenAnyLogoPresent,
logoPosition: optionalNumberWhenAnyLogoPresent,
cacheSeconds: Joi.number().integer().min(0),
style: Joi.string(),
})
Expand Down
1 change: 0 additions & 1 deletion core/base-service/base.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,6 @@ describe('BaseService', function () {
logo: undefined,
logoWidth: undefined,
logoSize: undefined,
logoPosition: undefined,
links: [],
labelColor: undefined,
cacheLengthSeconds: undefined,
Expand Down
18 changes: 3 additions & 15 deletions core/base-service/coalesce-badge.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ import toArray from './to-array.js'
// base64-encoded logos). Otherwise the default color is used. If the color
// is specified for a multicolor Shield logo, the named logo will be used and
// colored. The appearance of the logo can be customized using `logoWidth`,
// and in the case of the popout badge, `logoPosition`. When `?logo=` is
// specified, any logo-related parameters specified dynamically by the
// service, or by default in the service, are ignored.
// When `?logo=` is specified, any logo-related parameters specified
// dynamically by the service, or by default in the service, are ignored.
// 2. The second precedence is the dynamic logo returned by a service. This is
// used only by the Endpoint badge. The `logoColor` can be overridden by the
// query string.
Expand Down Expand Up @@ -56,7 +55,6 @@ export default function coalesceBadge(
} = overrides
let {
logoWidth: overrideLogoWidth,
logoPosition: overrideLogoPosition,
logoSize: overrideLogoSize,
color: overrideColor,
labelColor: overrideLabelColor,
Expand All @@ -78,7 +76,6 @@ export default function coalesceBadge(
overrideLabelColor = `${overrideLabelColor}`
}
overrideLogoWidth = +overrideLogoWidth || undefined
overrideLogoPosition = +overrideLogoPosition || undefined

const {
isError,
Expand All @@ -91,7 +88,6 @@ export default function coalesceBadge(
logoColor: serviceLogoColor,
logoSize: serviceLogoSize,
logoWidth: serviceLogoWidth,
logoPosition: serviceLogoPosition,
link: serviceLink,
cacheSeconds: serviceCacheSeconds,
style: serviceStyle,
Expand Down Expand Up @@ -122,12 +118,7 @@ export default function coalesceBadge(
style = 'flat'
}

let namedLogo,
namedLogoColor,
logoSize,
logoWidth,
logoPosition,
logoSvgBase64
let namedLogo, namedLogoColor, logoSize, logoWidth, logoSvgBase64
if (overrideLogo) {
// `?logo=` could be a named logo or encoded svg.
const overrideLogoSvgBase64 = decodeDataUrlFromQueryParam(overrideLogo)
Expand All @@ -143,7 +134,6 @@ export default function coalesceBadge(
// original width or position.
logoSize = overrideLogoSize
logoWidth = overrideLogoWidth
logoPosition = overrideLogoPosition
} else {
if (serviceLogoSvg) {
logoSvgBase64 = svg2base64(serviceLogoSvg)
Expand All @@ -156,7 +146,6 @@ export default function coalesceBadge(
}
logoSize = coalesce(overrideLogoSize, serviceLogoSize)
logoWidth = coalesce(overrideLogoWidth, serviceLogoWidth)
logoPosition = coalesce(overrideLogoPosition, serviceLogoPosition)
}
if (namedLogo) {
const iconSize = getIconSize(String(namedLogo).toLowerCase())
Expand Down Expand Up @@ -195,7 +184,6 @@ export default function coalesceBadge(
namedLogo,
logo: logoSvgBase64,
logoWidth,
logoPosition,
logoSize,
links: toArray(overrideLink || serviceLink),
cacheLengthSeconds: coalesce(serviceCacheSeconds, defaultCacheSeconds),
Expand Down
17 changes: 1 addition & 16 deletions core/base-service/coalesce-badge.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,13 @@ describe('coalesceBadge', function () {
).to.equal(getSimpleIcon({ name: 'npm', color: 'blue' })).and.not.be.empty
})

it("when the logo is overridden, it ignores the service's logo color, position, and width", function () {
it("when the logo is overridden, it ignores the service's logo color and width", function () {
expect(
coalesceBadge(
{ logo: 'npm' },
{
namedLogo: 'appveyor',
logoColor: 'red',
logoPosition: -3,
logoWidth: 100,
},
{},
Expand Down Expand Up @@ -288,20 +287,6 @@ describe('coalesceBadge', function () {
})
})

describe('Logo position', function () {
it('overrides the logoPosition', function () {
expect(coalesceBadge({ logoPosition: -10 }, {}, {})).to.include({
logoPosition: -10,
})
})

it('applies the logo position', function () {
expect(
coalesceBadge({}, { namedLogo: 'npm', logoPosition: -10 }, {}),
).to.include({ logoPosition: -10 })
})
})

describe('Links', function () {
it('overrides the links', function () {
expect(
Expand Down
1 change: 0 additions & 1 deletion core/base-service/legacy-request-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const globalQueryParams = new Set([
'link',
'logo',
'logoColor',
'logoPosition',
'logoSize',
'logoWidth',
'link',
Expand Down
7 changes: 6 additions & 1 deletion services/endpoint-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,14 @@ const endpointSchema = Joi.object({
logoSvg: Joi.string(),
logoColor: optionalStringWhenNamedLogoPresent,
logoWidth: optionalNumberWhenAnyLogoPresent,
logoPosition: optionalNumberWhenAnyLogoPresent,
style: Joi.string(),
cacheSeconds: Joi.number().integer().min(0),
/*
Retained for legacy compatibility
Although this does nothing,
passing it should not throw an error
*/
logoPosition: optionalNumberWhenAnyLogoPresent,
Copy link
Member Author

Choose a reason for hiding this comment

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

I've left this in the schema so it is not a breaking change

})
// `namedLogo` or `logoSvg`; not both.
.oxor('namedLogo', 'logoSvg')
Expand Down
9 changes: 0 additions & 9 deletions services/endpoint/endpoint.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,6 @@ The endpoint badge takes a single required query param: <code>url</code>, which
the query string.
</td>
</tr>
<tr>
<td><code>logoPosition</code></td>
<td>
Default: none. Same meaning as the query string. Can be overridden by
the query string.
</td>
</tr>
<tr>
<td><code>style</code></td>
<td>
Expand Down Expand Up @@ -157,7 +150,6 @@ export default class Endpoint extends BaseJsonService {
logoSvg,
logoColor,
logoWidth,
logoPosition,
style,
cacheSeconds,
}) {
Expand All @@ -171,7 +163,6 @@ export default class Endpoint extends BaseJsonService {
logoSvg,
logoColor,
logoWidth,
logoPosition,
style,
// don't allow the user to set cacheSeconds any shorter than this._cacheLength
cacheSeconds: Math.max(
Expand Down
18 changes: 18 additions & 0 deletions services/endpoint/endpoint.tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,24 @@ t.create('logoWidth')
logoWidth: 30,
})

// The logoPosition param was removed, but passing it should not
// throw a validation error. It should just do nothing.
t.create('logoPosition')
.get('.json?url=https://example.com/badge')
.intercept(nock =>
nock('https://example.com/').get('/badge').reply(200, {
schemaVersion: 1,
label: 'hey',
message: 'yo',
logoSvg,
logoPosition: 30,
}),
)
.expectBadge({
label: 'hey',
message: 'yo',
})

t.create('Invalid schema')
.get('.json?url=https://example.com/badge')
.intercept(nock =>
Expand Down
Loading