SAP / open-ux-tools

Enable community collaboration to jointly promote and facilitate best in class tooling capabilities
Apache License 2.0
83 stars 41 forks source link

BUG - Property change is created (unsaved) on key press #1568

Open IvoSG opened 11 months ago

IvoSG commented 11 months ago

Description

Property change is created (unsaved) on the key press but should be on blur for the adaptation projects When I start typing in a text field of the selected element, on every key press new property change is created.

Steps to Reproduce

Steps to reproduce the behavior:

  1. When I start typing in a text field of the selected element, on every key press new property change is created.
  2. Of course, these are unsaved, and on Save they condense, but it would be better if the change be created on leaving the field

Expected results

The change should be created on the blur event

Actual results

The changes are created on every key press event

Version/Components/Environment

Add any other context about the problem here OS:

Root Cause Analysis

Problem

{describe the problem}

Fix

{describe the fix}

Why was it missed

{Some explanation why this issue might have been missed during normal development/testing cycle}

How can we avoid this

{if we don’t want to see this type of issues anymore what we should do to prevent}

voicis commented 11 months ago

Why is this an issue for adaptation projects? Perhaps other scenarios are also affected.

I think we should keep the editing behaviours for all of the scenarios the same if possible, to be consistent and avoid causing confusion for the users.

IvoSG commented 10 months ago

@voicis the behavior is confusing in general case, not only in ADP scenarios. If you want we can apply the fix for all scenarios, not only the ones for ADP so the tool is consistent

voicis commented 10 months ago

It shouldn't create changes on every key stroke, only after 500ms when users stops typing. https://github.com/SAP/open-ux-tools/blob/63728bb6ab3b5e8ad8fd8010096bcdcad5f30fbc/packages/control-property-editor/src/panels/properties/StringEditor.tsx#L59. I just tested it and it still seemed to be working like that.

This was by design to save a few extra steps the user otherwise would need to take before they can see the changes on UI. The drawback is that you get more changes in the change panel.

In other tools where changes take effect on blur we also apply the change if the enter key is pressed, perhaps thats something we could also do here. Then the user doesn't need to focus out of the field and can explicitly apply the change and we don't have as many changes in the change panel.

cfg74 commented 9 months ago

I'm with @IvoSG. In addition of creating multiple changes this could easily result in inconsistencies as incomplete data is written to an (in memory) change file. We should harmonize the behaviour with how the property panel works in Page Editor, where input field changes are committed on enter and focus out.