frontend icon indicating copy to clipboard operation
frontend copied to clipboard

Clearable time selector

Open schelv opened this issue 2 years ago • 2 comments

Proposed change

The time selector can now be cleared: image

The same selector after pressing the clear button: 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:
  • 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:

schelv avatar Nov 09 '23 21:11 schelv

Once we get this would be good to add for duration selector as well, can fix #19360

karwosts avatar Jan 28 '24 15:01 karwosts

I think this will solve the issue where its not possible to clear a timeout without dropping back to YAML once its been set which seems like the root cause of https://github.com/home-assistant/core/issues/109586

It also resolves quandary in https://github.com/home-assistant/core/pull/115830 as it means it means we align the frontend to the backend

  • no timeout set (cleared), no timeout happens
  • 0 timeout set, times out right away

bdraco avatar Apr 19 '24 21:04 bdraco

@bramkragten I see you added the "Needs UX" label. Does that mean that some action needs to be taken for this merge request?

schelv avatar May 11 '24 12:05 schelv

@schelv thanks for your PR! We discussed this because it adds a nice extra feature to the input, but having the X outside the input is not up to standards. With a couple of tweaks, I think we can have the X inside the input.

Based on the M3 textfields, M3 time pickers and NN/g guidelines, I would suggest the following design. More details in the Figma File. I made it with the M3 Figma Kit, so the colors are wrong.

CleanShot 2024-07-03 at 11 54 28@2x

Changes

  • Move the AM/PM dropdown to a selector next to the input.
  • Show hour, minute and second labels when there is a value
  • Make the input label adjacent.
  • When empty, have a placeholder. Default can be No time, but I'm open for suggestions. Alternative can be to show -:-:-.

matthiasdebaat avatar Jul 03 '24 09:07 matthiasdebaat

Walkthrough

Walkthrough

The changes introduce a clearable property across the HaBaseTimeInput, HaTimeSelector, and HaTimeInput components, enhancing the user experience by allowing users to clear input values via a dedicated button. This update includes the addition of an icon for the button, conditional rendering based on the property, and necessary style adjustments for a cohesive appearance.

Changes

File Path Change Summary
src/components/ha-base-time-input.ts Added clearable property, implemented _clearValue method for the clear button functionality, and updated styles.
src/components/ha-selector/ha-selector-time.ts Introduced clearable property in the HaTimeSelector class.
src/components/ha-time-input.ts Introduced clearable property in the HaTimeInput class; modified _timeChanged method to handle clearing logic.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HaTimeInput
    participant HaBaseTimeInput
    participant HaTimeSelector
    
    User->>HaTimeInput: Trigger clear button
    HaTimeInput->>HaBaseTimeInput: Set value to undefined
    HaBaseTimeInput->>HaTimeSelector: Clear value
    HaTimeSelector-->>HaBaseTimeInput: Value cleared
    HaBaseTimeInput-->>HaTimeInput: Value cleared
    HaTimeInput-->>User: Value cleared

Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL

Commits

Files that changed from the base of the PR and between f78139545740d81f3f890c16ac973fb16cfd73eb and 274037e65230d0ac583ed76cc614ccd8738a27e7.

Files selected for processing (1)
  • src/components/ha-base-time-input.ts (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/components/ha-base-time-input.ts

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 Jul 13 '24 11:07 coderabbitai[bot]

@matthiasdebaat I've moved the X to be inside of the input, added labels, and separated the dropdown. I'm not the right person to implement the rest of the design. I have no idea what I'm doing.😅

image

schelv avatar Jul 13 '24 14:07 schelv

It's already a great improvement, thanks! Can you extend this line? CleanShot 2024-07-15 at 11 09 19@2x

matthiasdebaat avatar Jul 15 '24 09:07 matthiasdebaat

It's already a great improvement, thanks! Can you extend this line? CleanShot 2024-07-15 at 11 09 19@2x

I've tried to, but could not find the css property responsible for that line.

schelv avatar Jul 15 '24 09:07 schelv

@matthiasdebaat image 🎉

schelv avatar Jul 17 '24 18:07 schelv

YOU ROCK! Thanks!

matthiasdebaat avatar Jul 18 '24 07:07 matthiasdebaat

@matthiasdebaat , the current ux is good to go?

silamon avatar Jul 19 '24 07:07 silamon

@matthiasdebaat , the current ux is good to go?

One question, before it's good to go. Does this field have a adjacent label? Or do you need to add one above by yourself?

matthiasdebaat avatar Jul 19 '24 08:07 matthiasdebaat

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

home-assistant[bot] avatar Jul 19 '24 14:07 home-assistant[bot]

Whenever you're ready, feel free to hit the "ready to review" button. 🥇

silamon avatar Jul 24 '24 17:07 silamon