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

experimental: add transform section under style-panel with ui controls #3718

Merged
merged 28 commits into from
Jul 27, 2024

Conversation

JayaKrishnaNamburu
Copy link
Contributor

@JayaKrishnaNamburu JayaKrishnaNamburu commented Jul 13, 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

  • Need to discuss if the isExperiemental flag need to be removed before merging the branch.

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)
  • added tests
  • if any new env variables are added, added them to .env file

@JayaKrishnaNamburu
Copy link
Contributor Author

JayaKrishnaNamburu commented Jul 15, 2024

You can test the basic flow here @kof @TrySound there are definitely some UI changes and some code changes after merging with main.

The one thing that i think needs some attention is, should we add default values for all the transforms properties when they click "+" at the top of the section. And what happens when they are all added, because if they click again. It gets reset.

How can we show the transformOrigin perspective as layers as the others in translate, skew, rotate etc. And btw, i currently kept it behind a feature-flag just in case if we take this initial 4 properties as check-point and then do the rest in another PR.

Screen.Recording.2024-07-15.at.2.39.34.PM.mov

@JayaKrishnaNamburu
Copy link
Contributor Author

I have update the UX now, to follow the dropdown approach for adding individual transform properties and not everything at once. If the UX is looking good, let me know. I need to test things out once and write tests for the utils. And in the next MR, i will go for prespective, tranform-origin and other properties.

Screen.Recording.2024-07-17.at.1.54.28.PM.mov

@kof
Copy link
Member

kof commented Jul 17, 2024

UX is looking great!

Found a broken icon

image

@JayaKrishnaNamburu
Copy link
Contributor Author

Yeah, reagarding the broken icon. I tried re-exporting from figma several times. It still rendering as this. Any ideas on that ?

@kof
Copy link
Member

kof commented Jul 17, 2024

Please update all icons from here, when icon is not added to icons, never use it, it can have all kinds of unfinished stuff, we forgot to do it https://www.figma.com/design/sfCE7iLS0k25qCxiifQNLE/%F0%9F%93%9A-Webstudio-Library?node-id=2-1406&t=ZLITg8pprIprkKfb-0

@kof
Copy link
Member

kof commented Jul 17, 2024

Updated icons in the design doc as well

@JayaKrishnaNamburu
Copy link
Contributor Author

I see

fixtures/webstudio-remix-netlify-edge-functions/.webstudio/data.json

the fixtures project is updated. Someone should commit the updated fixtures to the main. The failing cli tests are not directly related to the changes in this one in that case.

@JayaKrishnaNamburu JayaKrishnaNamburu merged commit 5b46f79 into main Jul 27, 2024
11 of 13 checks passed
@JayaKrishnaNamburu JayaKrishnaNamburu deleted the transform-section branch July 27, 2024 07:20
@kof kof changed the title feat: add transform section under style-panel with ui controls experimental: add transform section under style-panel with ui controls Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants