Skip to content
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

Add shortcut hint to assist dialog #23739

Merged
merged 4 commits into from
Jan 20, 2025

Conversation

jpbede
Copy link
Member

@jpbede jpbede commented Jan 14, 2025

Proposed change

Add a shortcut hint to the assist dialog when it is opened via the topbar.

Opened via shortcut Opened via topbar
image image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • 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

Checklist

  • The code change is tested and works locally.
  • 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:

@silamon silamon added the Needs UX Pull requests requiring a review from the Home Assistant design team label Jan 14, 2025
@jpbede jpbede force-pushed the assist-shortcut-hint branch 2 times, most recently from f3f948a to 5c32def Compare January 16, 2025 08:00
@marcinbauer85
Copy link
Contributor

@jpbede This is a nice addition and aligned with other use cases in HA 👍

@wendevlin wendevlin removed the Needs UX Pull requests requiring a review from the Home Assistant design team label Jan 20, 2025
Copy link
Contributor

@wendevlin wendevlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jpbede!

@wendevlin wendevlin merged commit d121b33 into home-assistant:dev Jan 20, 2025
11 checks passed
@jpbede jpbede deleted the assist-shortcut-hint branch January 20, 2025 07:15
@balloob
Copy link
Member

balloob commented Jan 27, 2025

I don't think that this is not a good addition. The tip is big, takes up a lot of space and is shown every time. The tip at the bottom of the config screen is hidden off screen and only seen when you scroll all the way to the bottom.

I suggest we revert this and find a better way to teach this to users. For example, MacOS teaches users about shortcut keys in the menu that allows opening options:

image

@wendevlin
Copy link
Contributor

I agree @balloob, I merged it because of the Ok from Marcin.

@marcinbauer85 can you please have another look and tell us your pro/cons about it?

wendevlin added a commit that referenced this pull request Jan 28, 2025
balloob pushed a commit that referenced this pull request Jan 28, 2025
Revert "Add shortcut hint to assist dialog (#23739)"

This reverts commit d121b33.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assist dialog lacks keyboard shortcut tip underneath like other Quick Bar dialogs have
5 participants