-
Notifications
You must be signed in to change notification settings - Fork 25
fix: line-height of resource name #1209
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the line-height of resource names in the FilesList component by adding the leading-4 Tailwind CSS class. This addresses a visual issue with resource name display referenced in GitHub issue #937.
Key Changes
- Added
leading-4class to ensure consistent line-height for resource names - Applied the fix to both truncated and non-truncated display modes
- Updated test snapshots to reflect the CSS class changes
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/web-pkg/src/components/FilesList/ResourceName.vue | Added leading-4 class to spans containing resource names and extensions |
| packages/web-pkg/tests/unit/components/FilesList/snapshots/ResourceName.spec.ts.snap | Updated snapshots to include the new leading-4 class in test expectations |
| packages/web-pkg/tests/unit/components/FilesList/snapshots/ResourceListItem.spec.ts.snap | Updated snapshot to reflect the line-height fix in resource list item display |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@kulmann This fixes the vertical alignment of the resource names, especially in the top bar. It's still not 100% like before because the top bar height is now The buttons still look cramped just because there are too many, especially with the auto save icon. But this is no regression from Tailwind. Still, do we want to do anything about that? |
kulmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, looks good now 👍
I think we discussed recently that we could/should get rid of the autosave-icon by using a different save-icon... but for that we'd need some sort of icon stacking I guess. Do you have other ideas? |
The only other idea I have is to display an overlay in some corner during the process of auto-saving. But that's not an always visible indicator. Then again: how does the user benefit from this information? |
refs #937 (comment)