-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Script & Item edit: Add/Fix dirty handling #2374
Conversation
florian-h05
commented
Feb 18, 2024
•
edited
Loading
edited
- Add dirty handling for script-edit.
- Fix dirty handling of item-edit always saying dirty due to wrong initialization order.
- Code improvements to other dirty handlings.
@jimtng Can you please test this PR? |
Job #1651: Bundle Size — 11.01MiB (+0.02%).Warning Bundle contains 19 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
View job #1651 report View florian-h05:dirty branch activity View project dashboard |
I haven't worked on this, glad that you reminded me that this probably also need dirty handling. |
@jimtng FYI I have edited my last comment, what you tested is out of scope of this PR as I only focused on the actual pages, not dirty handling for popups. |
Fair enough. I wasn't aware that the other areas needed fixing (they of course did, evidenced by this PR). The popup was what was on my mind :) |
@jimtng It now became a bit bigger than expected, but now it works fine in my tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I found so far:
- Run Now will forcibly save current changes without prompting/warning
- After Run Now, edit the script again. This should make it dirty but it doesn't warn about the new changes when clicking Back out of the script editor. The changes made after clicking Run Now is lost!
bundles/org.openhab.ui/web/src/pages/settings/rules/script/script-edit.vue
Outdated
Show resolved
Hide resolved
I cannot reproduce that issue, for me this works fine. |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
This is still the case. Shouldn't we prompt user whether they wanted to save first before running?
It seems that this is not the case now. |
I would tent to say no, because I guess when you click "Run Now" you want to run what you have actually written, and not what is the saved state on the server. WDYT? |
I would like to be be prompted before it saves my changes. |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
I have added a dialog for that. |
I don't like the current behaviour. The prompt should say
When cancel is clicked, don't run. Right now it runs the old script prior to editing, which is not what I see on the screen, and that, to me, is confusing. |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Pushed 👍 |
Perfect! I've just noticed something - which is happening even before this PR:
This broke the address bar and the UI forcing me to do a page refresh to recover. Does this happen to you? |
Yes. I will merge this PR as it is unrelated to it, but it should be taken care of that. |
@florian-h05 I've just tried this:
|
I cannot reproduce this with the dev server on main (commit 69f4876). |
Regression from #2374 The problem is when I clicked on a file-based script in the UI, the content of the script didn't load. Before: <img width="601" alt="image" src="https://github.com/openhab/openhab-webui/assets/2554958/b3351f27-8ae6-4463-91ee-583fce0cf873"> After: <img width="670" alt="image" src="https://github.com/openhab/openhab-webui/assets/2554958/596629cb-459d-43c0-992e-7221e2a505dd"> Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>