-
Notifications
You must be signed in to change notification settings - Fork 10
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
make icons look better #461
Conversation
ping @jouni why do we need to override styles in lumo, shouldn't 'vaadin:' icons work out-of-the-box ? |
c952c65
to
ea24b57
Compare
I would rather have them work out of the box. |
I committed fixes for vaadin-button, vaadin-text-field and vaadin-tabs that should fix the issue. Can you try the master versions? |
@jouni those fixes work. Thanks We still need to merge this PR since it does a couple of additional things like setting correctly the icon attribute because the property is not reflected to the attribute, and improvement on how to show/hide the delivered icon. Also we need to wait for those fixes until there is a new release of the elements and vaadin platform including them. Review status: 0 of 4 files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
Reviewed 2 of 4 files at r1, 1 of 1 files at r2. Comments from Reviewable |
Review status: 3 of 4 files reviewed at latest revision, 1 unresolved discussion. src/main/webapp/frontend/src/views/storefront/order-status-badge.html, line 81 at r2 (raw file):
Am I right that this method will be called twice each time status is changed? Comments from Reviewable |
Review status: 3 of 4 files reviewed at latest revision, 1 unresolved discussion. src/main/webapp/frontend/src/views/storefront/order-status-badge.html, line 81 at r2 (raw file): Previously, alexberazouski wrote…
Only in the case that Comments from Reviewable |
Reviewed 1 of 4 files at r1. Comments from Reviewable |
Jira: BFF-634
This change is