-
Notifications
You must be signed in to change notification settings - Fork 18
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 css toggle-button ignores title translation #44
Conversation
Unfortunately, the current implementation ignores translations and the buttons texts are hardcoded inside the css. The easiest fix is to use the title attribute of the parent div that is already translated.
Thanks for your contribution. I will have a look. 👍 |
I like the solution and left just one small suggestion about code readability. Thanks for you contribution! :) |
width: 46px; | ||
height: 46px; | ||
line-height: 46px; | ||
padding: 10px; |
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.
This is not related to the translation issue, right?
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.
Yes it is. Fixed width does not work, otherwise you end up with an overflowing box and/or weird spacing. I replaced the line-height/height with padding for simplicity.
If you want to maintain the 46px height, just keep the line-height/height and only set padding to left/right.
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.
All right. So we should never have had fixed width here.
Co-authored-by: Maciej Barelkowski <maciejbarel@gmail.com>
I will be happy to merge this today, if you could only squash the changes and adjust the commit message to our contributing guidlines. Thanks! |
Already merged. Have a look at 038bb3c if you want to check what I meant :) And thanks for your contribution! |
Thx @barmac ❤️ |
Released in v2.0.4 :) |
Unfortunately, the current implementation ignores translations and the buttons texts are hardcoded inside the css.
The easiest fix is to use the title attribute of the parent div that is already translated.
Which issue does this PR address?
Closes #43