frontend icon indicating copy to clipboard operation
frontend copied to clipboard

Proposal for a new scene editor

Open karwosts opened this issue 1 year ago • 12 comments

Proposed change

A change to the current behavior of the scene editor is currently one of the most-voted requested features, and I spent a little time thinking about an alternative behavior that might be more user friendly and less surprising. I came up with the following:

In this proposal scenes are no longer activated simply because the scene editor is opened. Instead of yoking the config in the scene editor to the live status of all the devices in the scene, add a "capture" button for each device/entity that allows user to copy the current state of that device only into their scene. This allows decoupling the current state from the scene being edited.

By using this behavior, scene can be partially edited without having to take live the state of all other devices in the scene, and user can pick and choose which to update. Capture button also serves as an indicator that the current state of the device is not equal to the state saved in the scene.

This also adds an "Activate" button to the overflow menu, in case user wants to choose to activate the full scene.

As a possible enhancement, because the scene config state is decoupled from the live state, this would allow for adding a yaml editor here to allow for fine tweaking the scene state in yaml (similar to how automations elements can be edited in ui or yaml). But that has not been implemented yet.

Hasn't been tested that extensively yet, but just wanted to get an idea of if the UI might be something we would want to go for.

image

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] New feature (thank you!)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Example configuration


Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion: https://community.home-assistant.io/t/improve-scene-editor-allow-scene-edits-without-setting-devices-states/151053
  • Link to documentation pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [x] There is no commented out code in this PR.
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Summary by CodeRabbit

  • New Features
    • Enhanced scene management capabilities in the HaSceneEditor, allowing users to capture and manage changes to scene entities.
    • Introduced a feature to capture all dirty entities with a streamlined UI.
  • Improvements
    • Improved state management for unsaved changes, ensuring more responsive user interactions.
    • Updated localization with new strings for better clarity in the editor's functionality.

karwosts avatar Mar 15 '24 14:03 karwosts

Nice. IMO in overflow menu - activate I would replace the mdi:palette icon with mdi:play

WRoss7 avatar Mar 16 '24 13:03 WRoss7

The problem starts because the "more info" dialogs are being reused. I think virtual "more info" dialogs will be needed that allow to edit the scene state instead of the live state to prevent scenes to modify states.

To improve the user experience it would be great if the state, that the scene sets, can be seen in the scene editor instead of clicking it open. I find the visualization from a screenshot in #10064 very inspirational, although that's for the automation editor. image

I think the suggested changes will help but aside from these suggestions, this would be a big refactoring since there are multiple entity types.

silamon avatar Mar 20 '24 19:03 silamon

Nice idea.

I do have some questions:

  • Will pressing "Activate" couple the scene config to the live state? or only apply it?
  • If there will be a coupling of scene config and live state, how will it be visualized?
  • Will there also be an activate button for a single device?

schelv avatar Mar 31 '24 12:03 schelv

Nice idea.

I do have some questions:

  • Will pressing "Activate" couple the scene config to the live state? or only apply it?

Only calls scene.activate and nothing else. So no coupling.

  • If there will be a coupling of scene config and live state, how will it be visualized?

There isn't, so N/A.

  • Will there also be an activate button for a single device?

Did not add one in this proposal. Could maybe be added, wasn't convinced if it was necessary.

karwosts avatar Mar 31 '24 13:03 karwosts

I like the idea of not activating the scene immediately after you click on a scene, so you can see it, change the name, icon or whatever.

I'm not sure how I feel about individual capture buttons per device, would a new user understand this immediately? I can see the pros of this approach but...

What do you think about a "read-only" mode that is on by default and if you want to edit the scene it is still activated?

If we would go the way of this PR, I think the activate button, and an activate for a single device buttons should be way more prominent.

@Madelena can you pitch in here?

bramkragten avatar Apr 12 '24 18:04 bramkragten

What do you think about a "read-only" mode that is on by default and if you want to edit the scene it is still activated?

I admit that this is more complicated and may have a bit more of a learning curve, maybe design/ux team can come up with some ideas to make it prettier. I do think the ability to partial edit a scene can be really useful though, I wouldn't really like if the only way to change anything was to activate everything.

The usecase that came to mind was I recall reading about someone who had a scene for bedtime or something which put his computer into an off state. The scene could not be edited, as every time he wanted to edit it, it shutdown the computer on which he was trying to change it 😂

karwosts avatar Apr 12 '24 19:04 karwosts

Nice. IMO in overflow menu - activate I would replace the mdi:palette icon with mdi:play

It would match with what is shown in the other place: image

schelv avatar May 11 '24 12:05 schelv

@karwosts I think it might be a good idea to separate this PR into two PR's; one for the scene activation issue, another one for the capture buttons.

schelv avatar May 11 '24 12:05 schelv

I think it might be a good idea to separate this PR into two PR's; one for the scene activation issue, another one for the capture buttons.

I'm not sure the concepts can be easily separated. Anyway I am awaiting further concrete instruction from reviewers before taking further action.

karwosts avatar May 24 '24 16:05 karwosts

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Aug 22 '24 17:08 github-actions[bot]

Walkthrough

Walkthrough

The changes introduce enhancements to the HaSceneEditor class, particularly in state management for dirty entities, allowing for better tracking of unsaved changes. New methods for capturing the state of entities and devices have been added, alongside modifications to the UI to improve user interaction. Additionally, the JSON translation file has been updated with new keys to support localization for new UI actions related to scene management.

Changes

File Change Summary
src/panels/config/scene/ha-scene-editor.ts Updated HaSceneEditor class with new state management for dirty entities, added methods for capturing entities/devices, and improved _stateChanged method.
src/translations/en.json Added new keys "capture" and "capture_all" to support localized UI text for the editor section.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Aug 22 '24 17:08 coderabbitai[bot]

Unstale.

@Madelena your feedback was requested, do you think there is anything we can take from this to improve the scene editor?

karwosts avatar Aug 22 '24 17:08 karwosts

@karwosts we're working on a revised solution for this. I'll post it here today for review.

marcinbauer85 avatar Nov 13 '24 08:11 marcinbauer85

@karwosts after much debate we've opted for this design. It does differ from your original proposal but it targets the main problems of scenes:

  1. Viewing the scene
  2. Editing w/o applying I've left detailed notes on the design. Let me know how we can proceed. Thanks!

https://www.figma.com/design/TJ1yBZUyOWQtLmugi239BU/HA-12?node-id=0-1&t=3LfqlBPSrDVHV1pv-1

marcinbauer85 avatar Nov 13 '24 21:11 marcinbauer85

The new design didn't have everything that I initially wanted, but I think it's got enough in it to be interesting.

I've put up a new PR (#22847) with most of the requested changes.

Will close this one and continue there.

karwosts avatar Nov 16 '24 14:11 karwosts

❤️ @karwosts Yes we know, but it addresses the core problems with the scene editor right now. We'll continue to iterate on scenes in the future.

marcinbauer85 avatar Nov 18 '24 08:11 marcinbauer85