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

fix: better sorting of transitions and allow enter key to save #3968

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

JayaKrishnaNamburu
Copy link
Contributor

fixes #3958

Description

This PR fixes the enter key behaviour for the transition-property combo box. And we updated the sort mechanism at the time of searching to show the most related property at the top. So, users don't need to scroll all the way to see the matched property.

Steps for reproduction

  • Add a transition property layer.
  • Now, click on the layer and the UI panel to edit the properties opens up.
  • Type height or any other property in the combo box.
  • Now, press Enter. The action should save the value of the property and update the layer.
  • Next, search for some random property with a possibility for more results. Eg: width.
  • width or any other matching value that you searched for should always show up at the top of the list.

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)

@JayaKrishnaNamburu JayaKrishnaNamburu requested a review from kof August 20, 2024 06:03
@JayaKrishnaNamburu JayaKrishnaNamburu self-assigned this Aug 20, 2024
@JayaKrishnaNamburu JayaKrishnaNamburu changed the title fix: allow enter to save transition property fix: allow enter key to save transition property Aug 20, 2024
@kof kof changed the title fix: allow enter key to save transition property fix: better sorting of transitions + allow enter key to save Aug 20, 2024
@kof kof changed the title fix: better sorting of transitions + allow enter key to save fix: better sorting of transitions and allow enter key to save Aug 20, 2024
@kof
Copy link
Member

kof commented Aug 20, 2024

  • just found out that there is supposed to be some separation between the common section and the rest, now I don't seem to find any, what do I miss?

@JayaKrishnaNamburu
Copy link
Contributor Author

@kof looks like there might be some bug in the primitive itself. Because when the UI was initially built it was working good. Just cross check from old deployment. Will check once on the bug.
https://webstudio-builder-git-fix-transition-filtering-getwebstudio.vercel.app/builder/264a609f-ce63-4197-a364-68d239c2f56a
Screenshot 2024-08-20 at 6 56 31 PM

@johnsicili
Copy link
Contributor

@JayaKrishnaNamburu Enter works great! Thanks for the fix!

What's the point of having the checkmark if it's not selected by default, i.e., I can't click out without hitting enter or clicking it?

Kapture.2024-08-20.at.07.43.21.mp4

I think auto-highlighting it would be more accurate. Not a big deal, though, and if complex, don't worry about it.

@JayaKrishnaNamburu
Copy link
Contributor Author

https://webstudio-builder-git-fix-transition-filtering-getwebstudio.vercel.app/builder/264a609f-ce63-4197-a364-68d239c2f56a

I am not sure about the UX @johnsicili maybe we can open a new issue for the same to change on all places combo box is used.

@JayaKrishnaNamburu
Copy link
Contributor Author

Merging this, and opened a new issue to track the combobox issue with seperator
#3977

@JayaKrishnaNamburu JayaKrishnaNamburu merged commit 6a79e71 into main Aug 21, 2024
20 checks passed
@JayaKrishnaNamburu JayaKrishnaNamburu deleted the fix-transition-combobox branch August 21, 2024 03:26
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.

Changing transition property requires clicking it in dropdown, even if it shows a check
3 participants