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

CSS Transforms #3411

Closed
kof opened this issue May 23, 2024 · 11 comments · Fixed by #4057
Closed

CSS Transforms #3411

kof opened this issue May 23, 2024 · 11 comments · Fixed by #4057
Assignees
Labels
area:styles Style Panel is the CSS panel on the right complexity:high Multiple weeks of work or more prio:1 The most important thing to work on type:feat Features and enhancements

Comments

@kof
Copy link
Member

kof commented May 23, 2024

As a user I want to use CSS transforms

https://www.figma.com/design/xCBegXEWxROLqA1Y31z2Xo/%F0%9F%93%96-Webstudio-Design-Docs?node-id=10712-4063&m=dev

@kof kof added area:styles Style Panel is the CSS panel on the right type:feat Features and enhancements complexity:high Multiple weeks of work or more prio:1 The most important thing to work on labels May 23, 2024
@kof kof changed the title Transforms CSS Transforms May 23, 2024
@JayaKrishnaNamburu
Copy link
Contributor

JayaKrishnaNamburu commented Jun 27, 2024

I have some questions w.r.t to each section in the transform section. The UI is 100% achievable if only there is one-way direction of adding styles in the application. But in the whole application, there is always a space where users can add values using the code-editor text-area. And the values are parsed back into the editor and populated in style-panel. Let's see on by one.

Parsing the property value

transform is a very similar to filter which means all the possible functions are added to the same property without any , separated values like layers. So, first i thought we can just do exactly what we are doing in filters. But then after checking the UI, i realised we need to parse the value and group the functions into individual tuples again. We need to build a hierarchy on top of each filter function so we can show them as independnet layers.

Eg:
The typical way of doing this, just parsing everything and converting into data layer functions.

const transition: TupleValue = {
  type: "tuple",
  hidden: false,
  value: [
    {
      type: "function",
      name: "translate",
      args: {
        type: "layers",
        value: [{ type: "unit", unit: "px", value: 50 }],
      },
    },
    {
      type: "function",
      name: "rotate",
      args: {
        type: "layers",
        value: [{ type: "unit", unit: "deg", value: 50 }],
      },
    },
    {
      type: "function",
      name: "scale",
      args: {
        type: "layers",
        value: [
          { type: "keyword", value: "2" },
          { type: "keyword", value: "5" },
        ],
      },
    },
    {
      type: "function",
      name: "translateX",
      args: {
        type: "layers",
        value: [{ type: "unit", unit: "px", value: 50 }],
      },
    },
  ],
};

But this way, each function goes as an individual layer in the current layer list implementation. Like

- translate(50px)
- rotate(50deg)
- scale(2, 5)
- translateX(50px)

But the parser should parse them into

const transition: TupleValue = {
  type: "tuple",
  hidden: false,
  value: [
    {
      type: "tuple",
      value: [
        {
          type: "function",
          name: "translate",
          args: {
            type: "layers",
            value: [{ type: "unit", unit: "px", value: 50 }],
          },
        },
        {
          type: "function",
          name: "translateX",
          args: {
            type: "layers",
            value: [{ type: "unit", unit: "px", value: 50 }],
          },
        },
      ],
    },
    {
      type: "tuple",
      value: [
        {
          type: "function",
          name: "rotate",
          args: {
            type: "layers",
            value: [{ type: "unit", unit: "deg", value: 50 }],
          },
        },
      ],
    },
    {
      type: "tuple",
      value: [
        {
          type: "function",
          name: "scale",
          args: {
            type: "layers",
            value: [
              { type: "keyword", value: "2" },
              { type: "keyword", value: "5" },
            ],
          },
        },
      ],
    },
  ],
};

This will help to translate into translate(50px) translateX(50px) rotate(50deg) scale(2, 5) in deployment and this in UI

- translate(50px) translateX(50px)
- rotate(50deg)
- scale(2, 5)

and we can display the tabs accordingly. Now, let's see what other hurdles we might come across.

Translate

image

  • In the UI, there is option for translateX, translateY, translateZ. What happens when users add translate and translate3d.
  • Should they be parsed back into individual functions. Like translate(x, y) is equal to translateY and translateY respective.
  • But what happens if users add translate(10px, 5px) translateX(20px) in a single layer (given there is always possibility of multiple layers of translate). So, if a translate(0, 10) and translateY(20) present in same layer. Should they be processed into translateY(30) for that specific layer. To accommodate the UI. But this can he handled in a different way too. Instead of adding up. We can split into new layer. So, it's always a layer consists of translateX, translateY, translate3d and translate. And if the functions are repeating, it is pushed into a new layer.
  • But there is one thing we should always remember that doing these kind of operations will change the values in text-area where users can add. So, the app is basically modifying the users input so not exactly 1:1 thing here.

Rotate

image

  • Exactly all the changes as translate. But added rotate(50deg) is equal to rotateZ(50deg) as a single rotate value means rotating on z-axis and not x. So, the app should keep a check of these kind of special conversion too as the UI should strictly support only rotateX and rotateY.

We will have similar challenges with the other two functions like scale and skew too. If there is a decision for translate and rotate it can be applied to scale and skew too.

perspective and matrix

perspective and matrix too are valid transform functions. But in the UI there is no tab for them. Given users can have any number of these functions under transform like

transform: perspective(800px) perspective(50px)

These kind of issues are mainly because the panel allows for user inputs apart from the UI itself. So, during parsing, either we should always drop the perspective and matrix or add a new tab for that too. (imo: they should be supported and not left out because, iving text-area and then throwing errors for user-inputs is not a good UX).

image
In the UI, there is only a single control for perspective. So, it comes to same problem what happens if there are multiple that are being added using text-area.

@JayaKrishnaNamburu
Copy link
Contributor

cc @TrySound @kof would be great if you have some inputs on these.

@kof
Copy link
Member Author

kof commented Jun 27, 2024

  1. I would be fine if user pastes translate(x,y) to convert it to translateX() translateY() when saving @TrySound wdyt?
  2. regarding matrix - just open a panel with text editor only when clicking on that layer, treat anything we don't have UI for as a code-editable value

@TrySound
Copy link
Member

TrySound commented Jun 27, 2024

I would be fine if user pastes translate(x,y) to convert it to translateX() translateY() when saving @TrySound wdyt?

Doesn't really matter. We should interpret all kinds of input. Though the issue is these functions order affect rendering.

Seems like using shorthands like translate: , scale: and rotate: provide deterministic order by spec. We may want to rely on them instead of transform property
https://drafts.csswg.org/css-transforms-2/#ctm

Also interesting offset is treated as transform too
https://developer.mozilla.org/en-US/docs/Web/CSS/offset

@kof
Copy link
Member Author

kof commented Jun 27, 2024

@TrySound Can we honestly use "translate" property on a published site?
image

@TrySound
Copy link
Member

In 7 months will be green baseline but I can merge it in css engine as well

@TrySound
Copy link
Member

Other engines got it earlier

@JayaKrishnaNamburu
Copy link
Contributor

#3696 - parses individual translate property
#3704 - parses transform property

JayaKrishnaNamburu added a commit that referenced this issue Jul 11, 2024
## Description

Parses `translate` css property and converts them into tuples. Related
to the series of PR that enables transform property #3411

## 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

- [x] made a self-review

## Before merging

- [x] tested locally and on preview environment (preview dev login:
5de6)
- [x] added tests
JayaKrishnaNamburu added a commit that referenced this issue Jul 27, 2024
## Description

Adds the primary `translate`, `scale`, `rotate`, skew` properties under
transform section. Related to #3411

## Steps for reproduction
- Add multiple properties under the transform section.
- Update values of each property from the UI.
- Hide/Delete each individual property

## Todo
- [x] Need to discuss if the `isExperiemental` flag need to be removed
before merging the branch.

## Code Review

- [x] 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

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

## Before merging

- [x] tested locally and on preview environment (preview dev login:
5de6)
- [x] added tests
- [ ] if any new env variables are added, added them to `.env` file

---------

Co-authored-by: Oleg Isonen <oleg008@gmail.com>
@johnsicili
Copy link
Contributor

There's a gap between each transform that's not consistent with other sections with layers fyi.

Screenshot 2024-07-27 at 1 56 11 PM

@JayaKrishnaNamburu
Copy link
Contributor

There's a gap between each transform that's not consistent with other sections with layers fyi.

Screenshot 2024-07-27 at 1 56 11 PM

@johnsicili its fixed in this one 👍🏽

#3802

@johnsicili
Copy link
Contributor

Sorry.. reacted with the wrong emoji.. on mobile the laughing face looked like a smile. This looks really good btw, can't wait for it to be released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:styles Style Panel is the CSS panel on the right complexity:high Multiple weeks of work or more prio:1 The most important thing to work on type:feat Features and enhancements
Projects
Status: 🌐 Released
Development

Successfully merging a pull request may close this issue.

4 participants