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

Improve add cards dialog user experience #23773

Merged
merged 11 commits into from
Jan 21, 2025

Conversation

jpbede
Copy link
Member

@jpbede jpbede commented Jan 17, 2025

Proposed change

Improve the add cards dialog user experience based on the Figma proposal from the product team: #23717

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
Copy link
Contributor

silamon commented Jan 17, 2025

The height of the dialog content is a bit tweaked so it fits. Alternatively, you can have a sticky search bar (top 0), force the background the same way you did as the card container title. Then to finish off, use sticky card container title with top set to 56px being the height of the search input.

@jpbede
Copy link
Member Author

jpbede commented Jan 17, 2025

Thnx @silamon for that great suggestion. This is indeed way cleaner 🙏

@wendevlin
Copy link
Contributor

Nice 👍

I don't like that the title wrappers are so edgy. We could just have a background with round corners aground the title, the rest of the title line is transparent.

image

Moreover what about align the card to top of the wrapper? When there is a big card on the row a small card can be far away from the title. I would see it easier on the top of the wrapper. @marcinbauer85

image

@jpbede
Copy link
Member Author

jpbede commented Jan 20, 2025

I don't like that the title wrappers are so edgy. We could just have a background with round corners aground the title, the rest of the title line is transparent.

I like that idea 👍

@marcinbauer85
Copy link
Contributor

@wendevlin @jpbede let's please use a gradient background instead to soften the transition:
background: linear-gradient(180deg, #ffffffff 0%, #ffffff00 80%); (apply also for dark theme).
Also, let's stay with vertical alignment: center. Both top and center have issues, and we want to implement changes step by step.

@jpbede One additional thing that you could fix here is that it's impossible to tab through this dialogs content and select a card by keyboard alone, hence making this not accessible for people relying on only keyboards or screen readers.
Could you look into this, please?
Side note: using tab can now cycle to a preview-card UI element and control it, which isn't something we want here.

@wendevlin
Copy link
Contributor

@jpbede When you search the result styles are broken. I saw you have removed the card-container around search results, this leads to the issue.

@jpbede
Copy link
Member Author

jpbede commented Jan 21, 2025

Oh okay, will look into it, thanks.

@jpbede
Copy link
Member Author

jpbede commented Jan 21, 2025

I moved the card container around the sections, so the card container headers wont stack and forgotten to add it around the search results

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.

👍

@wendevlin wendevlin merged commit e994e35 into home-assistant:dev Jan 21, 2025
15 checks passed
@jpbede jpbede deleted the improve-add-card-dialog branch January 21, 2025 09:26
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.

4 participants