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 CB discarding changes if pressed Ctrl + Enter too quickly #6875

Merged
merged 7 commits into from
Jun 1, 2023

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented May 29, 2023

Pull Request Description

Now, quick typing in component browser and pressing "enter" should not cut off the last part typed. Fixes #6733

ugi-bugi-2023-05-29_15.00.35.mp4

On this occasion, also fixed "go-to-dashboard" button and "Unsupported engine version" being over the full-screen visualization. Fixes #6722

Important Notes

I did a significant refactoring of Project View:

  1. The huge frp::extend block was split into multiple init methods.
  2. Remaining of the "Old searcher" were removed.
  3. The "Edited" event from node's input is emitted only when in edit mode (it's consistent with other API terminology, and makes FRP for showing CB much simpler.

The code was mostly moved around, but the check is advised anyway, as there were small changes here and there.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@farmaazon farmaazon added the CI: No changelog needed Do not require a changelog entry for this PR. label May 29, 2023
@farmaazon farmaazon self-assigned this May 29, 2023
@Frizi
Copy link
Contributor

Frizi commented May 30, 2023

QA report: 🔴

  1. When entering edit mode on a node containing a type constructor, the component browser incorrectly selects the type itself as an option. Pressing enter then enters into that type, and confirming it again results in an arbitrary other method (not constructor in this case) being selected.
cb-double-confirm.mp4
  1. When selecting a type suggestion, component browser immediately switches selection to a different entry.
cb-type-select.mp4

@farmaazon
Copy link
Contributor Author

  1. It's on develop, but I have a quickfix and try to add it here.
  2. It's also on develop. It was partially worked around recently, so after last merge you should not have a problem in this specific scenario. The full fix is tracked by Cursor keys when editing a node move both the caret position and the selected item in the Component Browser #6124

@farmaazon
Copy link
Contributor Author

@Frizi fix landed, please check out.

@Frizi
Copy link
Contributor

Frizi commented May 31, 2023

QA Status: 🍌

Ctrl+enter works correctly, It will always accept currently written input. The issues mentioned above are now fixed.

There is still an issue related to input lag when simply using enter, as the component browser takes a while to respond to the input, and the accepted suggestion is effectively outdated at the time of key press. I'm going to say it is a separate issue, and likely needs further general performance improvements to be adressed properly. Here is how it looks though:

cb-quick-type.mp4

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label May 31, 2023
@farmaazon farmaazon added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Jun 1, 2023
@mergify mergify bot merged commit 2e39d75 into develop Jun 1, 2023
@mergify mergify bot deleted the wip/ao/fix-quick-ctrl-enter branch June 1, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
3 participants