frontend icon indicating copy to clipboard operation
frontend copied to clipboard

Published 20 hours ago • verified publisher icon home-assistant

Add a picture uploader to picture-card-editor

Open karwosts opened this issue 2 years ago • 12 comments

Proposed change

Add a picture-upload form to the picture-card editor. I think this makes working with images much more accessible, otherwise it can be a source of a lot of confusion how to add pictures to the server and figure out the path.

Maybe this should be wrapped up in some kind of image-selector element that can be reused?

That would allow for either entering an external URL, uploading, or maybe opening a file picker and allowing to select from the list of current uploaded images either in /image/ or /www/?

image-picker

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://github.com/home-assistant/frontend/discussions/18669
  • 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:

karwosts avatar Nov 18 '23 17:11 karwosts

Oh I do like this!

Maybe this should be wrapped up in some kind of image-selector element that can be reused?

Yes, that would be a good idea, similar to the file selector

That would allow for either entering an external URL, uploading, or maybe opening a file picker and allowing to select from the list of current uploaded images either in /image/ or /www/?

That also sounds good, but should be options on the selector then probably? The last option would need backend support first. (We do need something similar for uploaded pictures though probably, as there is no way to manage or reuse already uploaded images)

bramkragten avatar Nov 19 '23 12:11 bramkragten

If we had image-selector, what do you think of how the current UI looks?

OK to just have a <ha-picture-upload> stacked on top of a textbox for the path. Or is that take up too much space? Maybe have to click a button to show the uploader?

Image browser/picker could come later if/when that ever gets implemented.

karwosts avatar Nov 19 '23 14:11 karwosts

I like this one too! I'm seeing 3 options to select an image, correct me if I'm wrong:

  • Select a local file using image URL
  • Upload image from device, that converts to a local image URL
  • Upload image from web, that converts to a local image URL

In my opinion, the input-select with image URL isn't user friendly and pretty advanced. You have to know the path to a file, that requires access to your system. Can we drop this one? That means there should be an option to select an image from the system, like we're doing with the Media panel.

matthiasdebaat avatar Nov 20 '23 09:11 matthiasdebaat

I like this one too! I'm seeing 3 options to select an image, correct me if I'm wrong:

  • Select a local file using image URL

The URL input can also be remote URL on the web, it doesn't have to be local.

karwosts avatar Nov 20 '23 13:11 karwosts

I think I would hide the input by default, but allow the user to switch to it with a button

bramkragten avatar Nov 22 '23 13:11 bramkragten

I like this one too! I'm seeing 3 options to select an image, correct me if I'm wrong:

  • Select a local file using image URL
  • Upload image from device, that converts to a local image URL
  • Upload image from web, that converts to a local image URL

The URL input can also be remote URL on the web, it doesn't have to be local.

That is the third option in my mind. So it would be:

  • Upload image from web, that converts web URL to a local image URL

matthiasdebaat avatar Nov 22 '23 14:11 matthiasdebaat

I think I would hide the input by default, but allow the user to switch to it with a button

I agree. There should be a way to select between selecting a local (already uploaded) file, upload new image or upload image from the web. This last option only have an input. The other doesn't need one in my mind.

matthiasdebaat avatar Nov 22 '23 14:11 matthiasdebaat

I think I would hide the input by default

Can you clarify, what is "the input" you're referring to hide, the text selector, or the picture-upload?

I was kind of thinking to have the button toggle the larger upload element, though I'm wondering if Matthias is suggesting the opposite; to hide the URL. I'm not sure if that makes sense to me yet.

This is kind of what I was going to go for:

picture-card-2

Matthias, you seem to also be suggesting that remote URLs should be replaced with "upload to a local image from web", but I think people might not necessarily want to upload remote URLs. The behavior now is just to show the live image from the remote domain. Maybe that is desired, in case the remote image is dynamic, or user just don't want to store it on his server?

karwosts avatar Nov 22 '23 15:11 karwosts

@matthiasdebaat can you check above questions?

bramkragten avatar Nov 27 '23 14:11 bramkragten

Thanks for your feedback. Based on this I've got something like this in my mind. More details can be found in the Figma design.

CleanShot 2023-12-07 at 10 59 49@2x

My suggestions are:

  • Let the user to choose between upload or URL. When the user chooses upload, the URL input is hidden and vice versa.
  • Add a delete button when an image is uploaded.
  • URL is as we currently have it.

matthiasdebaat avatar Dec 07 '23 10:12 matthiasdebaat

Updated from feedback.

I've for now dropped the crop feature from this as I don't really like how it works. When crop is enabled you are forced to go through the crop dialog, and there's no easy way to just use the entire image unaltered (you would have to try to drag the crop bubbles to exactly the corner of your image, and probably get it off by 1 pixel), so that seems potentially frustrating.

A better flow might be to have first the entire picture uploaded without crop, and then have a CROP button on the line below the picture to run it through the crop flow? Maybe that could be a different change.

image-selector-3

karwosts avatar Dec 07 '23 20:12 karwosts

We should add the camera entity and image entity to that radio list, as that will override the image (the entity will too btw on a picture-entity card, if it is a camera entity... 😓 )

bramkragten avatar Feb 28 '24 12:02 bramkragten

Walkthrough

Walkthrough

The updates introduce a new image selector component (ha-selector-image) and enhance image handling across various parts of the application. Changes include adding style support, updating image selection mechanisms in different card editors, and modifying image-related data handling functions. Additionally, new translations and styles for warning buttons have been incorporated.

Changes

Files/Groups Change Summary
src/components/ha-picture-upload.ts Added haStyle import, wrapped ha-button in a div, modified styles method to include haStyle.
src/components/ha-selector/ha-selector-image.ts Introduced ha-selector-image component with image upload and URL selection features.
src/components/ha-selector/ha-selector.ts Added ha-selector-image to LOAD_ELEMENTS for image selection functionality.
src/data/image_upload.ts Added URL prefix constant, function to extract ID from URL, renamed parameter in deleteImage function.
src/data/selector.ts Introduced ImageSelector interface in Selector type.
src/panels/lovelace/editor/config-elements/... Updated image property selector from text to image in various card editors (picture, entity, glance).
src/resources/styles.ts Added new style rule for warning buttons, setting primary theme color to error color.
src/translations/en.json Added new "image" section with keys for selecting, uploading, and specifying image URLs.

Sequence Diagram(s) (Beta)

sequenceDiagram
    participant User
    participant HaImageSelector
    participant HaPictureUpload
    participant ImageUploadAPI

    User->>HaImageSelector: Select image option
    HaImageSelector->>User: Show upload/URL input
    User->>HaImageSelector: Upload image / Enter URL
    HaImageSelector->>HaPictureUpload: Upload image
    HaPictureUpload->>ImageUploadAPI: Send image data
    ImageUploadAPI->>HaPictureUpload: Confirm upload
    HaPictureUpload->>HaImageSelector: Return image URL
    HaImageSelector->>User: Display selected image
Recent Review Details

Configuration used: CodeRabbit UI Review profile: ASSERTIVE

Commits Files that changed from the base of the PR and between 5fab1969a8d85b5ac4fe9dcc34d66d7f77fb6482 and aaaf3aff8ce0a030870ae803f7a85fd29a197493.
Files selected for processing (10)
  • src/components/ha-picture-upload.ts (3 hunks)
  • src/components/ha-selector/ha-selector-image.ts (1 hunks)
  • src/components/ha-selector/ha-selector.ts (1 hunks)
  • src/data/image_upload.ts (2 hunks)
  • src/data/selector.ts (2 hunks)
  • src/panels/lovelace/editor/config-elements/hui-picture-card-editor.ts (1 hunks)
  • src/panels/lovelace/editor/config-elements/hui-picture-entity-card-editor.ts (1 hunks)
  • src/panels/lovelace/editor/config-elements/hui-picture-glance-card-editor.ts (1 hunks)
  • src/resources/styles.ts (1 hunks)
  • src/translations/en.json (2 hunks)
Additional Context Used
Biome (58)
src/components/ha-picture-upload.ts (5)

136-136: Unexpected any. Specify a different type.

1-2: Some named imports are only used as types.

7-11: Some named imports are only used as types.

11-12: All these imports are only used as types.

110-110: Use Number.NaN instead of the equivalent global.

src/components/ha-selector/ha-selector-image.ts (4)

18-18: Unexpected any. Specify a different type.

1-1: Some named imports are only used as types.

3-4: All these imports are only used as types.

4-5: All these imports are only used as types.

src/components/ha-selector/ha-selector.ts (5)

72-72: Unexpected any. Specify a different type.

81-81: Unexpected any. Specify a different type.

87-87: Unexpected any. Specify a different type.

1-1: Some named imports are only used as types.

4-9: Some named imports are only used as types.

src/data/image_upload.ts (4)

18-18: This variable implicitly has the any type.

32-32: This type annotation is trivially inferred from its initialization.

58-60: This else clause can be omitted because previous branches break early.

1-1: All these imports are only used as types.

src/data/selector.ts (20)

339-339: Unexpected any. Specify a different type.

363-363: Unexpected any. Specify a different type.

470-484: Prefer for...of instead of forEach.

486-499: Prefer for...of instead of forEach.

501-512: Prefer for...of instead of forEach.

525-539: Prefer for...of instead of forEach.

553-566: Prefer for...of instead of forEach.

567-578: Prefer for...of instead of forEach.

590-601: Prefer for...of instead of forEach.

688-688: Forbidden non-null assertion.

783-785: Forbidden non-null assertion.

811-813: Forbidden non-null assertion.

4-5: All these imports are only used as types.

5-6: All these imports are only used as types.

6-10: Some named imports are only used as types.

10-14: All these imports are only used as types.

14-15: All these imports are only used as types.

91-91: Don't use '{}' as a type.

103-103: Don't use '{}' as a type.

108-108: Don't use '{}' as a type.

src/panels/lovelace/editor/config-elements/hui-picture-card-editor.ts (8)

78-78: Forbidden non-null assertion.

80-80: Forbidden non-null assertion.

85-85: Forbidden non-null assertion.

88-88: Forbidden non-null assertion.

4-5: All these imports are only used as types.

6-7: All these imports are only used as types.

7-8: All these imports are only used as types.

9-10: All these imports are only used as types.

src/panels/lovelace/editor/config-elements/hui-picture-entity-card-editor.ts (4)

120-120: Forbidden non-null assertion.

122-122: Forbidden non-null assertion.

126-126: Forbidden non-null assertion.

1-1: Some named imports are only used as types.

src/panels/lovelace/editor/config-elements/hui-picture-glance-card-editor.ts (8)

82-82: Forbidden non-null assertion.

86-86: Forbidden non-null assertion.

122-122: Change to an optional chain.

135-135: Forbidden non-null assertion.

137-137: Forbidden non-null assertion.

141-141: Forbidden non-null assertion.

145-145: Forbidden non-null assertion.

1-1: Some named imports are only used as types.

Additional comments not posted (10)
src/data/image_upload.ts (2)

11-11: The addition of URL_PREFIX is a good practice for centralizing API endpoints, making them easier to manage and modify.

78-78: Renaming the parameter from media_id to image_id in the deleteImage function improves clarity, aligning the terminology with the rest of the image handling functions.

src/panels/lovelace/editor/config-elements/hui-picture-card-editor.ts (1)

27-27: Updating the schema to use the new image selector aligns with the PR's goal to enhance image handling. This change facilitates a more intuitive UI for image selection.

src/panels/lovelace/editor/config-elements/hui-picture-entity-card-editor.ts (1)

35-35: The addition of the image selector to the schema is consistent with the PR's enhancements to image handling, providing a unified approach across different card editors.

src/components/ha-picture-upload.ts (1)

Line range hint 5-174: The updates to HaPictureUpload include the integration of haStyle and improvements to the UI for handling image uploads and changes. The implementation of the image change and upload functionalities is clear and aligns with the PR's objectives to enhance image management.

src/panels/lovelace/editor/config-elements/hui-picture-glance-card-editor.ts (1)

38-38: The update to include the image selector in the schema for the picture glance card editor is consistent with the changes across other card editors, enhancing the user's ability to manage images directly within the UI.

src/components/ha-selector/ha-selector.ts (1)

35-35: Addition of image selector type to support new image selection functionality.

This change integrates the new ha-selector-image component into the selector system, enabling image selection capabilities across the platform.

src/resources/styles.ts (1)

85-85: Styling for warning buttons to use error color.

This change ensures that warning buttons are visually consistent with error indicators, enhancing UI consistency and user experience.

src/data/selector.ts (1)

43-43: Introduction of ImageSelector type to support new image selection functionality.

This addition is crucial for enabling the new image selector functionality within the system, allowing for a structured way to handle image selections.

src/translations/en.json (1)

380-384: Translations for the new image handling features are clear and well-structured.

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 Configration 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 May 29 '24 12:05 coderabbitai[bot]

I would still really like to combine the image entity/camera entity/upload image/image url options for improved clarity that they can not be used together. But this is already a nice addition that we should merge. Thanks @karwosts!

bramkragten avatar May 29 '24 12:05 bramkragten