-
Notifications
You must be signed in to change notification settings - Fork 78
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
feat(input-time): calcite-input-time component #5639
Changes from 1 commit
6655f20
4605d3f
2ebcae9
a27557f
70b845b
ab46531
d174eaf
8cc29a6
a733397
3f79747
fe74b2f
4d90238
5b09e6b
d0eeb50
83c3b07
5f3bd02
fb96021
ed151ec
f581c2d
d7b2e34
6f0b9fa
a2c490c
17c53ff
e4dfbf6
8c3de9b
a35f3fe
cf5a2c4
e72b740
363f5df
010aa1f
1695a0b
4f40b5d
9135667
3afd715
d840e27
4995c82
3038b29
a29e090
ae4cbf5
85d0834
fd02493
cebc33f
68d638b
b685fd0
6159644
102fa15
c32aa1b
cd2d261
660ef17
f06f8f3
7e6fa0d
e0ceff4
a431d44
91842fb
4a5ac10
8131538
57a1504
b4f64da
87eed03
cdb3b66
4178d39
550814a
ec8eeb1
9554cf4
f2d8bdb
9d66f9d
cc4e559
c9bbff9
301cda4
7423586
2a1ac41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -231,11 +231,6 @@ export class InputTime | |
// | ||
//-------------------------------------------------------------------------- | ||
|
||
/** | ||
* @internal | ||
*/ | ||
@Event() calciteInternalInputTimeBlur: EventEmitter<void>; | ||
|
||
/** | ||
* Fires when the time value is committed. | ||
*/ | ||
|
@@ -246,6 +241,11 @@ export class InputTime | |
*/ | ||
@Event() calciteInputTimeInput: EventEmitter<string>; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
@Event() calciteInternalInputTimeBlur: EventEmitter<void>; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
|
@@ -259,10 +259,29 @@ export class InputTime | |
|
||
@Listen("blur") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to create some documentation alignment on how we are handling event listeners. I prefer this syntax but other devs are saying these should be setup as on event listeners in the component render itself. @benelan thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I don't feel strongly in either direction but I agree on the need for consistency. I'll run it by Franco when he returns. |
||
blurHandler(): void { | ||
this.activeEl = undefined; | ||
this.calciteInternalInputTimeBlur.emit(); | ||
this.emitChangeIfUserModified(); | ||
} | ||
|
||
@Listen("click") | ||
clickHandler(event: MouseEvent): void { | ||
const composedEventPath = event.composedPath(); | ||
if (composedEventPath.includes(this.hourEl)) { | ||
return; | ||
} | ||
if (composedEventPath.includes(this.minuteEl)) { | ||
return; | ||
} | ||
if (composedEventPath.includes(this.secondEl)) { | ||
return; | ||
} | ||
if (composedEventPath.includes(this.meridiemEl)) { | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use one if statement for all of these, it seems overly verbose. Maybe just a bunch of if ([this.hourEl, this.minuteEl, this.secondEl, this.meridiemEl].some((el) =>
composedEventPath.includes(el))) return;
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the performance difference is very slight however so in this case I would defer to which seems more readable. |
||
this.setFocus(); | ||
} | ||
|
||
@Listen("focus") | ||
focusHandler(): void { | ||
this.calciteInternalInputTimeFocus.emit(); | ||
|
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 should be
<void>
too