Skip to content

Commit 21a6a9d

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Decouple property renderers from StylePropertyTreeElement
This CL decouples the property renderers from StylePropertyTreeElement as much as possible. Rendering a value trace will involve rendering subtrees (for substitutions) which correspond to different tree elements. Making the renderers independent of it simplifies that. Full decoupling is impossible because editing properties through a popover (e.g., the color picker) strictly needs access. Therefore, editing and showing popovers is gated behind a StylePropertyTreeElement being passed. Bug: 396080529 Change-Id: I9a3e27c49641d52f81fd43bc386681ca98b5f432 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6286228 Commit-Queue: Philip Pfaffe <[email protected]> Reviewed-by: Ergün Erdoğmuş <[email protected]>
1 parent b143710 commit 21a6a9d

File tree

4 files changed

+211
-180
lines changed

4 files changed

+211
-180
lines changed

front_end/panels/elements/CSSValueTraceView.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ async function showTrace(
7272
}));
7373
await renderPromise.promise;
7474
renderPromise = Promise.withResolvers<Elements.CSSValueTraceView.ViewInput>();
75-
view.showTrace(property, matchedStyles, new Map(), treeElement.getPropertyRenderers());
75+
view.showTrace(
76+
property, matchedStyles, new Map(),
77+
Elements.StylePropertyTreeElement.getPropertyRenderers(
78+
property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement,
79+
treeElement.getComputedStyles() ?? new Map()));
7680
return await renderPromise.promise;
7781
}
7882

front_end/panels/elements/StylePropertyTreeElement.test.ts

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
3131
let fakeComputeCSSVariable: SinonStub<
3232
[style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, variableName: string],
3333
SDK.CSSMatchedStyles.CSSVariableValue|null>;
34-
35-
async function setUpCSSModel() {
36-
stubNoopSettings();
37-
setMockConnectionResponseHandler('CSS.enable', () => ({}));
38-
const cssModel = new SDK.CSSModel.CSSModel(createTarget());
39-
await cssModel.resumeModel();
40-
const domModel = cssModel.domModel();
41-
const node = new SDK.DOMModel.DOMNode(domModel);
42-
node.id = 0 as Protocol.DOM.NodeId;
43-
LegacyUI.Context.Context.instance().setFlavor(SDK.DOMModel.DOMNode, node);
44-
return {cssModel};
45-
}
34+
let cssModel: SDK.CSSModel.CSSModel;
4635

4736
beforeEach(async () => {
4837
const computedStyleModel = new Elements.ComputedStyleModel.ComputedStyleModel();
@@ -73,6 +62,15 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
7362
new Bindings.ResourceMapping.ResourceMapping(SDK.TargetManager.TargetManager.instance(), workspace);
7463
Bindings.CSSWorkspaceBinding.CSSWorkspaceBinding.instance(
7564
{forceNew: true, resourceMapping, targetManager: SDK.TargetManager.TargetManager.instance()});
65+
66+
stubNoopSettings();
67+
setMockConnectionResponseHandler('CSS.enable', () => ({}));
68+
cssModel = new SDK.CSSModel.CSSModel(createTarget());
69+
await cssModel.resumeModel();
70+
const domModel = cssModel.domModel();
71+
const node = new SDK.DOMModel.DOMNode(domModel);
72+
node.id = 0 as Protocol.DOM.NodeId;
73+
LegacyUI.Context.Context.instance().setFlavor(SDK.DOMModel.DOMNode, node);
7674
});
7775

7876
function addProperty(name: string, value: string, longhandProperties: Protocol.CSS.CSSProperty[] = []) {
@@ -125,7 +123,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
125123
});
126124

127125
it('is able to expand longhands with vars', async () => {
128-
await setUpCSSModel();
129126
setMockConnectionResponseHandler(
130127
'CSS.getLonghandProperties', (request: Protocol.CSS.GetLonghandPropertiesRequest) => {
131128
if (request.shorthandName !== 'shorthand') {
@@ -298,17 +295,13 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
298295

299296
describe('jumping to animations panel', () => {
300297
let domModel: SDK.DOMModel.DOMModel;
301-
beforeEach(() => {
298+
beforeEach(async () => {
302299
const target = createTarget();
303300
const domModelBeforeAssertion = target.model(SDK.DOMModel.DOMModel);
304301
assert.exists(domModelBeforeAssertion);
305302
domModel = domModelBeforeAssertion;
306303
});
307304

308-
afterEach(() => {
309-
sinon.reset();
310-
});
311-
312305
it('should render a jump-to icon when the animation with the given name exists for the node', async () => {
313306
const stubAnimationGroup = sinon.createStubInstance(SDK.AnimationModel.AnimationGroup);
314307
const getAnimationGroupForAnimationStub =
@@ -1592,7 +1585,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
15921585

15931586
describe('LengthRenderer', () => {
15941587
it('shows a popover with pixel values for relative units', async () => {
1595-
await setUpCSSModel();
15961588
setMockConnectionResponseHandler(
15971589
'CSS.resolveValues',
15981590
(request: Protocol.CSS.ResolveValuesRequest) =>
@@ -1617,7 +1609,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16171609

16181610
describe('MathFunctionRenderer', () => {
16191611
it('strikes out non-selected values', async () => {
1620-
await setUpCSSModel();
16211612
setMockConnectionResponseHandler(
16221613
'CSS.resolveValues',
16231614
(request: Protocol.CSS.ResolveValuesRequest) => ({
@@ -1641,7 +1632,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16411632

16421633
describe('AutoBaseRenderer', () => {
16431634
it('strikes out non-selected values', async () => {
1644-
await setUpCSSModel();
16451635

16461636
const stylePropertyTreeElement = getTreeElement('display', '-internal-auto-base(inline, block)');
16471637

@@ -1667,7 +1657,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16671657
beforeEach(async () => {
16681658
promptStub = sinon.stub(Elements.StylesSidebarPane.CSSPropertyPrompt.prototype, 'initialize').resolves([]);
16691659

1670-
const {cssModel} = await setUpCSSModel();
16711660
const gridNode = LegacyUI.Context.Context.instance().flavor(SDK.DOMModel.DOMNode);
16721661
const currentNode = new SDK.DOMModel.DOMNode(cssModel.domModel());
16731662
currentNode.id = 1 as Protocol.DOM.NodeId;

0 commit comments

Comments
 (0)