-
Notifications
You must be signed in to change notification settings - Fork 0
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
Input element #24
Input element #24
Conversation
import '@vaadin/vaadin-lumo-styles/color.js'; | ||
import '@vaadin/vaadin-lumo-styles/sizing.js'; | ||
import '@vaadin/vaadin-lumo-styles/spacing.js'; | ||
import '@vaadin/vaadin-lumo-styles/style.js'; |
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.
Do we have some convention that should all of these be imported always or only those that you refer to in the same file? Spacing seems to be the only one in use below.
package.json
Outdated
"@vaadin/vaadin-element-mixin": "^2.4.1", | ||
"@vaadin/vaadin-lumo-styles": "^1.6.1", | ||
"@vaadin/vaadin-material-styles": "^1.3.2", | ||
"@vaadin/vaadin-text-field": "^3.0.0", |
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.
Please use ^2.8.0
if you want the component to be compatible with Vaadin 14.x
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.
Done.
src/vaadin-message-input.js
Outdated
static get is() { | ||
return 'vaadin-message-input'; | ||
} | ||
|
||
static get version() { | ||
return '1.0.0-alpha1'; | ||
} | ||
} |
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.
nit: please place these getters after static get template
and before ready
method.
That's the convention that we follow in other components.
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.
Done.
Co-authored-by: Pekka Maanpää <pekkamaa@vaadin.com>
To be Vaadin 14 compatible.
These didn't come automatically via text-area anymore after downgrade from 3.0 to 2.8
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.
LGTM for my comments and to unlock merging after other comments are resolved.
It was allready deleted in the actual class.
Dismissing my own review as I took over fixing the review issues.
Things that need to be configurable: