-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
fix: Fix Notification title word break #15008
Conversation
Deploy preview for element ready! Built with commit 5c176a1 |
@iamkun, I don't think that this is an appropriate fix. You can't just split words up for everyone's notifications. It looks bad, and I know my client will complain about this. The correct solution would be to adjust the padding so that the text never reaches the X. I can sort this is if you want. Please re-open my original issue. |
@breadadams agreed |
Cool @iamkun, it's looking better! However given the right amount of characters the text could still run just against the side of the ✖️ icon: I'd add ~8px of padding-right to |
I think this 2px fix would be enough for most cases. 😬 |
Sure thing @iamkun, could be... I couldn't actually get it to happen with anything in english other than random characters, however a client of mine managed to with a Spanish string. Let's see how the 2px fix works and if necessary I'll open a hotfix 😉 |
Very good point @wacky6, I'd never tried a notification without a title. The right padding needs increasing a bit more. From a UI point of view I think it's better with a bigger gap like in the above screenshot, it separates the concerns of the "content" and the "action area" better (imo). Currently the close button is closer to the text than the icon, I had a client trying to click the red X icon because they didn't see the close button - true story. So maybe changing from |
@breadadams @wacky6 Updated based on reviews. 😀 |
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.
Looks good to me @iamkun 👏
close #14893
Please make sure these boxes are checked before submitting your PR, thank you!
dev
branch.