Bug 1575529: Display color values for manifest members r=Ola

Differential Revision: https://phabricator.services.mozilla.com/D45829

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Belén Albeza 2019-09-18 10:03:44 +00:00
parent b82fefd873
commit e0a909f7ea
15 changed files with 389 additions and 32 deletions

View File

@ -11,6 +11,7 @@
* Components
*/
@import "resource://devtools/client/application/src/components/App.css";
@import "resource://devtools/client/application/src/components/manifest/ManifestColorItem.css";
@import "resource://devtools/client/application/src/components/manifest/ManifestIssue.css";
@import "resource://devtools/client/application/src/components/manifest/ManifestIssueList.css";
@import "resource://devtools/client/application/src/components/manifest/ManifestItem.css";

View File

@ -20,11 +20,14 @@ const FluentReact = require("devtools/client/shared/vendor/fluent-react");
const Localized = createFactory(FluentReact.Localized);
const { l10n } = require("../../modules/l10n");
const ManifestColorItem = createFactory(require("./ManifestColorItem"));
const ManifestItem = createFactory(require("./ManifestItem"));
const ManifestIssueList = createFactory(require("./ManifestIssueList"));
const ManifestSection = createFactory(require("./ManifestSection"));
const ManifestJsonLink = createFactory(require("./ManifestJsonLink"));
const { MANIFEST_MEMBER_VALUE_TYPES } = require("../../constants");
/**
* A canonical manifest, splitted in different sections
*/
@ -56,6 +59,16 @@ class Manifest extends PureComponent {
: null;
}
renderMember({ key, value, type }) {
switch (type) {
case MANIFEST_MEMBER_VALUE_TYPES.COLOR:
return ManifestColorItem({ label: key, key, value });
case MANIFEST_MEMBER_VALUE_TYPES.STRING:
default:
return ManifestItem({ label: key, key }, value);
}
}
renderItemSections() {
const { identity, icons, presentation } = this.props;
@ -74,18 +87,7 @@ class Manifest extends PureComponent {
// NOTE: this table should probably be its own component, to keep
// the same level of abstraction as with the validation issues
// Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1577138
table(
{},
tbody(
{},
// TODO: handle different data types for values (colors, images…)
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1575529
items.map(item => {
const { key, value } = item;
return ManifestItem({ label: key, key: key }, value);
})
)
)
table({}, tbody({}, items.map(this.renderMember)))
);
});
}

View File

@ -0,0 +1,22 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
.manifest-item__color {
/* NOTE: platform converts any color format that is in the manifest to
hexadecimal, so we can uppercase */
text-transform: uppercase;
}
.manifest-item__color::before {
display: inline-block;
content: '';
background-color: var(--color-value); /* injected via React */
border-radius: 50%;
width: calc(var(--base-unit) * 3);
height: calc(var(--base-unit) * 3);
margin-block-start: calc(var(--base-unit) * -0.5);
margin-inline-end: var(--base-unit);
box-shadow: 0 0 0 1px var(--theme-splitter-color);
vertical-align: middle;
}

View File

@ -0,0 +1,46 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
const {
createFactory,
PureComponent,
} = require("devtools/client/shared/vendor/react");
const { span } = require("devtools/client/shared/vendor/react-dom-factories");
const ManifestItem = createFactory(require("./ManifestItem"));
/**
* This component displays a Manifest member which holds a color value
*/
class ManifestColorItem extends PureComponent {
static get propTypes() {
return {
label: PropTypes.string.isRequired,
value: PropTypes.string,
};
}
renderColor() {
const { value } = this.props;
return value
? span(
{
className: "manifest-item__color",
style: { "--color-value": value },
},
value
)
: null;
}
render() {
const { label } = this.props;
return ManifestItem({ label }, this.renderColor());
}
}
module.exports = ManifestColorItem;

View File

@ -4,6 +4,8 @@
DevToolsModules(
'Manifest.js',
'ManifestColorItem.css',
'ManifestColorItem.js',
'ManifestEmpty.js',
'ManifestIssue.css',
'ManifestIssue.js',

View File

@ -33,6 +33,11 @@ const MANIFEST_CATEGORIES = {
ICONS: "icons",
};
const MANIFEST_MEMBER_VALUE_TYPES = {
STRING: "string",
COLOR: "color",
};
const MANIFEST_ISSUE_LEVELS = {
ERROR: "error",
WARNING: "warning",
@ -46,6 +51,7 @@ module.exports = Object.assign(
PAGE_TYPES,
MANIFEST_CATEGORIES,
MANIFEST_ISSUE_LEVELS,
MANIFEST_MEMBER_VALUE_TYPES,
},
actionTypes
);

View File

@ -7,6 +7,7 @@
const {
MANIFEST_CATEGORIES,
MANIFEST_ISSUE_LEVELS,
MANIFEST_MEMBER_VALUE_TYPES,
FETCH_MANIFEST_FAILURE,
FETCH_MANIFEST_START,
FETCH_MANIFEST_SUCCESS,
@ -40,6 +41,16 @@ function _processRawManifestMembers(rawManifest) {
}
}
function getValueTypeForMember(key) {
switch (key) {
case "theme_color":
case "background_color":
return MANIFEST_MEMBER_VALUE_TYPES.COLOR;
default:
return MANIFEST_MEMBER_VALUE_TYPES.STRING;
}
}
const res = {
[MANIFEST_CATEGORIES.IDENTITY]: [],
[MANIFEST_CATEGORIES.PRESENTATION]: [],
@ -52,7 +63,8 @@ function _processRawManifestMembers(rawManifest) {
for (const [key, value] of rawMembers) {
const category = getCategoryForMember(key);
res[category].push({ key, value });
const type = getValueTypeForMember(key);
res[category].push({ key, value, type });
}
return res;

View File

@ -85,10 +85,45 @@ const MULTIPLE_WORKER_MIXED_DOMAINS_LIST = [
// props for a simple manifest
const MANIFEST_SIMPLE = {
icons: [{ key: "1x1", value: "something.png" }],
identity: [{ key: "name", value: "foo" }],
identity: [{ key: "name", value: "foo", type: "string" }],
presentation: [
{ key: "lorem", value: "ipsum" },
{ key: "foo", value: "bar" },
{ key: "lorem", value: "ipsum", type: "string" },
{ key: "foo", value: "bar", type: "string" },
],
validation: [{ level: "warning", message: "This is a warning" }],
};
const MANIFEST_STRING_MEMBERS = {
icons: [],
identity: [{ key: "name", value: "foo", type: "string" }],
presentation: [],
validation: [],
};
// props for a manifest with color values
const MANIFEST_COLOR_MEMBERS = {
icons: [],
identity: [],
presentation: [
{ key: "background_color", value: "red", type: "color" },
{ key: "theme_color", value: "rgb(0, 0, 0)", type: "color" },
],
validation: [],
};
const MANIFEST_UNKNOWN_TYPE_MEMBERS = {
icons: [],
identity: [{ key: "lorem", value: "ipsum", type: "foo" }],
presentation: [],
validation: [],
};
const MANIFEST_WITH_ISSUES = {
icons: [{ key: "1x1", value: "something.png" }],
identity: [{ key: "name", value: "foo", type: "string" }],
presentation: [
{ key: "lorem", value: "ipsum", type: "string" },
{ key: "foo", value: "bar", type: "string" },
],
validation: [{ level: "warning", message: "This is a warning" }],
};
@ -96,10 +131,10 @@ const MANIFEST_SIMPLE = {
// props for a manifest with no validation issues
const MANIFEST_NO_ISSUES = {
icons: [{ key: "1x1", value: "something.png" }],
identity: [{ key: "name", value: "foo" }],
identity: [{ key: "name", value: "foo", type: "string" }],
presentation: [
{ key: "lorem", value: "ipsum" },
{ key: "foo", value: "bar" },
{ key: "lorem", value: "ipsum", type: "string" },
{ key: "foo", value: "bar", type: "string" },
],
validation: [],
};
@ -109,7 +144,11 @@ module.exports = {
SINGLE_WORKER_DEFAULT_DOMAIN_LIST,
SINGLE_WORKER_DIFFERENT_DOMAIN_LIST,
MANIFEST_NO_ISSUES,
MANIFEST_WITH_ISSUES,
MANIFEST_SIMPLE,
MANIFEST_COLOR_MEMBERS,
MANIFEST_STRING_MEMBERS,
MANIFEST_UNKNOWN_TYPE_MEMBERS,
MULTIPLE_WORKER_LIST,
MULTIPLE_WORKER_MIXED_DOMAINS_LIST,
};

View File

@ -66,7 +66,7 @@ exports[`Manifest does not render the issues section when the manifest is valid
</article>
`;
exports[`Manifest renders the expected snapshot for a simple manifest 1`] = `
exports[`Manifest does render the issues section when the manifest is not valid 1`] = `
<article
className="js-manifest"
>
@ -146,3 +146,145 @@ exports[`Manifest renders the expected snapshot for a simple manifest 1`] = `
</ManifestSection>
</article>
`;
exports[`Manifest renders the expected snapshot for a manifest with color members 1`] = `
<article
className="js-manifest"
>
<Localized
id="manifest-view-header"
>
<h1
className="app-page__title"
/>
</Localized>
<ManifestJsonLink />
<ManifestSection
key="manifest-section-1"
title="manifest-item-identity"
>
<table>
<tbody />
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-2"
title="manifest-item-presentation"
>
<table>
<tbody>
<ManifestColorItem
key="background_color"
label="background_color"
value="red"
/>
<ManifestColorItem
key="theme_color"
label="theme_color"
value="rgb(0, 0, 0)"
/>
</tbody>
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-3"
title="manifest-item-icons"
>
<table>
<tbody />
</table>
</ManifestSection>
</article>
`;
exports[`Manifest renders the expected snapshot for a manifest with string members 1`] = `
<article
className="js-manifest"
>
<Localized
id="manifest-view-header"
>
<h1
className="app-page__title"
/>
</Localized>
<ManifestJsonLink />
<ManifestSection
key="manifest-section-1"
title="manifest-item-identity"
>
<table>
<tbody>
<ManifestItem
key="name"
label="name"
>
foo
</ManifestItem>
</tbody>
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-2"
title="manifest-item-presentation"
>
<table>
<tbody />
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-3"
title="manifest-item-icons"
>
<table>
<tbody />
</table>
</ManifestSection>
</article>
`;
exports[`Manifest renders the expected snapshot for a manifest with unknown types 1`] = `
<article
className="js-manifest"
>
<Localized
id="manifest-view-header"
>
<h1
className="app-page__title"
/>
</Localized>
<ManifestJsonLink />
<ManifestSection
key="manifest-section-1"
title="manifest-item-identity"
>
<table>
<tbody>
<ManifestItem
key="lorem"
label="lorem"
>
ipsum
</ManifestItem>
</tbody>
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-2"
title="manifest-item-presentation"
>
<table>
<tbody />
</table>
</ManifestSection>
<ManifestSection
key="manifest-section-3"
title="manifest-item-icons"
>
<table>
<tbody />
</table>
</ManifestSection>
</article>
`;

View File

@ -0,0 +1,24 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`ManifestColorItem renders the expected snapshot for a populated color item 1`] = `
<ManifestItem
label="foo"
>
<span
className="manifest-item__color"
style={
Object {
"--color-value": "#ff0000",
}
}
>
#ff0000
</span>
</ManifestItem>
`;
exports[`ManifestColorItem renders the expected snapshot for an empty color item 1`] = `
<ManifestItem
label="foo"
/>
`;

View File

@ -33,6 +33,7 @@ exports[`ManifestPage renders the expected snapshot when there is a manifest 1`]
Array [
Object {
"key": "name",
"type": "string",
"value": "foo",
},
]
@ -41,10 +42,12 @@ exports[`ManifestPage renders the expected snapshot when there is a manifest 1`]
Array [
Object {
"key": "lorem",
"type": "string",
"value": "ipsum",
},
Object {
"key": "foo",
"type": "string",
"value": "bar",
},
]

View File

@ -12,8 +12,11 @@ const Manifest = createFactory(
);
const {
MANIFEST_COLOR_MEMBERS,
MANIFEST_STRING_MEMBERS,
MANIFEST_UNKNOWN_TYPE_MEMBERS,
MANIFEST_NO_ISSUES,
MANIFEST_SIMPLE,
MANIFEST_WITH_ISSUES,
} = require("../fixtures/data/constants");
/*
@ -21,11 +24,31 @@ const {
*/
describe("Manifest", () => {
it("renders the expected snapshot for a simple manifest", () => {
const wrapper = shallow(Manifest(MANIFEST_SIMPLE));
it("renders the expected snapshot for a manifest with string members", () => {
const wrapper = shallow(Manifest(MANIFEST_STRING_MEMBERS));
expect(wrapper).toMatchSnapshot();
});
it("renders the expected snapshot for a manifest with color members", () => {
const wrapper = shallow(Manifest(MANIFEST_COLOR_MEMBERS));
expect(wrapper).toMatchSnapshot();
});
it("renders the expected snapshot for a manifest with unknown types", () => {
const wrapper = shallow(Manifest(MANIFEST_UNKNOWN_TYPE_MEMBERS));
expect(wrapper).toMatchSnapshot();
});
it("does render the issues section when the manifest is not valid", () => {
const wrapper = shallow(Manifest(MANIFEST_WITH_ISSUES));
expect(wrapper).toMatchSnapshot();
const sections = wrapper.find("ManifestSection");
expect(sections).toHaveLength(4);
expect(sections.get(0).props.title).toBe("manifest-item-warnings");
expect(sections.find("ManifestIssueList")).toHaveLength(1);
});
it("does not render the issues section when the manifest is valid", () => {
const wrapper = shallow(Manifest(MANIFEST_NO_ISSUES));
expect(wrapper).toMatchSnapshot();
@ -33,6 +56,6 @@ describe("Manifest", () => {
const sections = wrapper.find("ManifestSection");
expect(sections).toHaveLength(3);
expect(sections.get(0).props.title).not.toBe("manifest-item-warnings");
expect(sections.contains("ManifestIssueList")).toBe(false);
expect(sections.find("ManifestIssueList")).toHaveLength(0);
});
});

View File

@ -0,0 +1,30 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
// Import libs
const { shallow } = require("enzyme");
const { createFactory } = require("react");
const ManifestColorItem = createFactory(
require("devtools/client/application/src/components/manifest/ManifestColorItem")
);
/*
* Unit tests for the ManifestItem component
*/
describe("ManifestColorItem", () => {
it("renders the expected snapshot for a populated color item", () => {
const wrapper = shallow(
ManifestColorItem({ label: "foo", value: "#ff0000" })
);
expect(wrapper).toMatchSnapshot();
});
it("renders the expected snapshot for an empty color item", () => {
const wrapper = shallow(ManifestColorItem({ label: "foo" }));
expect(wrapper).toMatchSnapshot();
});
});

View File

@ -7,7 +7,7 @@
const { shallow } = require("enzyme");
const { createFactory } = require("react");
const ManifestSection = createFactory(
const ManifestItem = createFactory(
require("devtools/client/application/src/components/manifest/ManifestItem")
);
@ -17,12 +17,12 @@ const ManifestSection = createFactory(
describe("ManifestItem", () => {
it("renders the expected snapshot for a populated item", () => {
const wrapper = shallow(ManifestSection({ label: "foo" }, "bar"));
const wrapper = shallow(ManifestItem({ label: "foo" }, "bar"));
expect(wrapper).toMatchSnapshot();
});
it("renders the expected snapshot for an empty item", () => {
const wrapper = shallow(ManifestSection({ label: "foo" }));
const wrapper = shallow(ManifestItem({ label: "foo" }));
expect(wrapper).toMatchSnapshot();
});
});

View File

@ -8,8 +8,11 @@ const {
FETCH_MANIFEST_START,
FETCH_MANIFEST_SUCCESS,
RESET_MANIFEST,
MANIFEST_MEMBER_VALUE_TYPES,
} = require("devtools/client/application/src/constants.js");
const { STRING, COLOR } = MANIFEST_MEMBER_VALUE_TYPES;
const {
manifestReducer,
ManifestState,
@ -25,7 +28,7 @@ const MANIFEST_PROCESSING = [
{
source: { name: "Foo" },
processed: {
identity: [{ key: "name", value: "Foo" }],
identity: [{ key: "name", value: "Foo", type: STRING }],
},
},
// manifest with two members from the same category
@ -36,8 +39,8 @@ const MANIFEST_PROCESSING = [
},
processed: {
identity: [
{ key: "short_name", value: "Short Foo" },
{ key: "name", value: "Long Foo" },
{ key: "short_name", value: "Short Foo", type: STRING },
{ key: "name", value: "Long Foo", type: STRING },
],
},
},
@ -48,8 +51,10 @@ const MANIFEST_PROCESSING = [
background_color: "#FF0000",
},
processed: {
identity: [{ key: "name", value: "Foo" }],
presentation: [{ key: "background_color", value: "#FF0000" }],
identity: [{ key: "name", value: "Foo", type: STRING }],
presentation: [
{ key: "background_color", value: "#FF0000", type: COLOR },
],
},
},
// manifest with icons