Skip to content

Commit

Permalink
Do not show flex properties in style panel when layout is not flex (#722
Browse files Browse the repository at this point in the history
)

## Description

1. What is this PR about (link the issue and add a short description)

Do not show flex properties in the style panel when a layout is not flex
closes #687

<img width="272" alt="image"
src="https://user-images.githubusercontent.com/5077042/210067036-4571be1d-4745-48a3-be7f-45f91791a6a9.png">

Here I patched `appliesTo` of properties `alignItems`, `justifyItems`
see mdn/data#585

## Steps for reproduction

Opens designer panel, check no flex props in display: block layout

## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [ ] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [ ] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio-designer/blob/main/apps/designer/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env.example`
and the `designer/env-check.js` if mandatory
  • Loading branch information
istarkov authored Jan 2, 2023
1 parent eacbd7d commit d73ec1b
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,26 @@ export const dependencies: Dependencies = {
},
flexContainers: {
property: "display",
values: ["flex"],
values: ["flex", "inline-flex"],
},
// @todo this should actually check on parent
flexItemsAndInFlowPseudos: {
property: "display",
values: ["flex"],
values: ["flex", "inline-flex"],
},
// @todo needs to also check flex-wrap
multilineFlexContainers: {
property: "flexWrap",
values: ["wrap", "wrap-reverse"],
},
multiColumnElementsFlexContainersGridContainers: {
property: "display",
values: ["flex", "inline-flex", "grid"],
},
flexContainersGridContainers: {
property: "display",
values: ["flex", "inline-flex", "grid"],
},

// Used by alignSelf
// @todo needs to check parent to be display: flex or grid, position: absolute
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const appliesTo = (
currentStyle: StyleInfo
): boolean => {
const { appliesTo } = styleConfig;

if (appliesTo in dependencies) {
const dependency = dependencies[appliesTo];
if (dependency === undefined) {
Expand Down
26 changes: 24 additions & 2 deletions packages/css-data/bin/mdn-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,25 @@ const propertiesData: {
};
} = {};

const patchAppliesTo = (property: Property, config: Value) => {
// see https://github.com/mdn/data/issues/585 alignItems and justifyItems have appliesTo = "allElements"
// this specification https://www.w3.org/TR/css-align-3/ - "block containers", "grid containers", "flex containers"
// chrome devtools check grid or flex here https://github.com/ChromeDevTools/devtools-frontend/blob/354fb0fd3fc0a4af43ef760450e7d644d0e04daf/front_end/panels/elements/CSSRuleValidator.ts#L374
// our opinion is that it must be "grid containers", "flex containers"
if (property === "align-items" || property === "justify-items") {
if (config.appliesto !== "allElements") {
throw new Error(
"Specification has changed, please check and update the code"
);
}

// flexContainersGridContainers not exists in mdn-data, it's our custom value
return "flexContainersGridContainers";
}

return config.appliesto;
};

let property: Property;

for (property in filteredProperties) {
Expand Down Expand Up @@ -141,7 +160,8 @@ for (property in filteredProperties) {
popularity:
popularityIndex.find((data) => data.property === property)
?.dayPercentage || 0,
appliesTo: config.appliesto,

appliesTo: patchAppliesTo(property, config),
};
}

Expand Down Expand Up @@ -176,7 +196,9 @@ const keywordValues = (() => {

// When there is syntax - there are keyword references
if (syntax) {
if (parsedSyntaxes.has(syntax)) return parsedSyntaxes.get(syntax);
if (parsedSyntaxes.has(syntax)) {
return parsedSyntaxes.get(syntax);
}
const ast = definitionSyntax.parse(syntax);
definitionSyntax.walk(ast, (node) => {
keywords = new Set([...keywords, ...getKeywords(node)]);
Expand Down
2 changes: 1 addition & 1 deletion packages/css-data/src/__generated__/keyword-values.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions packages/css-data/src/__generated__/properties.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/css-data/src/__generated__/units.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d73ec1b

Please sign in to comment.