-
Notifications
You must be signed in to change notification settings - Fork 79
fix: ensure beforeOpen
/open
and beforeClose
/close
events emit properly
#9958
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
Merged
jcfranco
merged 15 commits into
dev
from
jcfranco/9641-fix-open-close-related-events-when-target-has-multiple-transitioned-props
Aug 14, 2024
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
6c9fe06
fix: ensure open/close events emit correctly if target element has mu…
jcfranco c5310b0
fix copypasta
jcfranco ea2b4bf
revisit timing test approach to avoid protocol errors when events are…
jcfranco bdfb459
fix tooltip test assertions
jcfranco 859fb5a
increase duration for animated cases to ensure assertions
jcfranco ffd188a
drop redundant alert transition item (targets all)
jcfranco cbf1743
fix block open/close events
jcfranco 3f79d4a
fix notice open/close events
jcfranco 37f705b
fix input-time-picker open/close events
jcfranco 9e6786a
adjust minimum delay used between animation and non-animation tests
jcfranco 6f96d21
fix block tests
jcfranco 10b539e
fix focus trap regression
jcfranco 4914e95
tidy up
jcfranco d7250c8
fix initially open input-time-picker use case
jcfranco 8662334
ensure initially open tests are in a separate describe block and upda…
jcfranco File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,6 @@ import { | |
updateMessages, | ||
} from "../../utils/t9n"; | ||
import { getSupportedLocale } from "../../utils/locale"; | ||
import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent"; | ||
import { decimalPlaces } from "../../utils/math"; | ||
import { getIconScale } from "../../utils/component"; | ||
import { Validation } from "../functional/Validation"; | ||
|
@@ -169,7 +168,6 @@ export class InputTimePicker | |
LabelableComponent, | ||
LoadableComponent, | ||
LocalizedComponent, | ||
OpenCloseComponent, | ||
T9nComponent | ||
{ | ||
//-------------------------------------------------------------------------- | ||
|
@@ -179,19 +177,17 @@ export class InputTimePicker | |
//-------------------------------------------------------------------------- | ||
|
||
/** When `true`, displays the `calcite-time-picker` component. */ | ||
|
||
@Prop({ reflect: true, mutable: true }) open = false; | ||
|
||
@Watch("open") | ||
openHandler(): void { | ||
onToggleOpenCloseComponent(this); | ||
|
||
if (this.disabled || this.readOnly) { | ||
this.open = false; | ||
return; | ||
} | ||
|
||
this.reposition(true); | ||
// we set the property instead of the attribute to ensure popover's open/close events are emitted properly | ||
this.popoverEl.open = this.open; | ||
} | ||
|
||
/** When `true`, interaction is prevented and the component is displayed with lower opacity. */ | ||
|
@@ -395,10 +391,6 @@ export class InputTimePicker | |
|
||
private referenceElementId = `input-time-picker-${guid()}`; | ||
|
||
openTransitionProp = "opacity"; | ||
|
||
transitionEl: HTMLCalciteInputElement; | ||
|
||
//-------------------------------------------------------------------------- | ||
// | ||
// State | ||
|
@@ -559,21 +551,42 @@ export class InputTimePicker | |
// | ||
// -------------------------------------------------------------------------- | ||
|
||
onBeforeOpen(): void { | ||
private popoverBeforeOpenHandler = (event: CustomEvent<void>): void => { | ||
event.stopPropagation(); | ||
this.calciteInputTimePickerBeforeOpen.emit(); | ||
} | ||
}; | ||
|
||
onOpen(): void { | ||
private popoverOpenHandler = (event: CustomEvent<void>): void => { | ||
event.stopPropagation(); | ||
this.calciteInputTimePickerOpen.emit(); | ||
} | ||
|
||
onBeforeClose(): void { | ||
activateFocusTrap(this, { | ||
onActivate: () => { | ||
if (this.focusOnOpen) { | ||
this.calciteTimePickerEl.setFocus(); | ||
this.focusOnOpen = false; | ||
} | ||
}, | ||
}); | ||
}; | ||
|
||
private popoverBeforeCloseHandler = (event: CustomEvent<void>): void => { | ||
event.stopPropagation(); | ||
this.calciteInputTimePickerBeforeClose.emit(); | ||
} | ||
}; | ||
|
||
onClose(): void { | ||
private popoverCloseHandler = (event: CustomEvent<void>): void => { | ||
event.stopPropagation(); | ||
this.calciteInputTimePickerClose.emit(); | ||
} | ||
|
||
deactivateFocusTrap(this, { | ||
onDeactivate: () => { | ||
this.calciteInputEl.setFocus(); | ||
this.focusOnOpen = false; | ||
}, | ||
}); | ||
this.open = false; | ||
}; | ||
|
||
syncHiddenFormInput(input: HTMLInputElement): void { | ||
syncHiddenFormInput("time", this, input); | ||
|
@@ -685,27 +698,6 @@ export class InputTimePicker | |
return timeString; | ||
} | ||
|
||
private popoverCloseHandler = () => { | ||
deactivateFocusTrap(this, { | ||
onDeactivate: () => { | ||
this.calciteInputEl.setFocus(); | ||
this.focusOnOpen = false; | ||
}, | ||
}); | ||
this.open = false; | ||
}; | ||
|
||
private popoverOpenHandler = () => { | ||
activateFocusTrap(this, { | ||
onActivate: () => { | ||
if (this.focusOnOpen) { | ||
this.calciteTimePickerEl.setFocus(); | ||
this.focusOnOpen = false; | ||
} | ||
}, | ||
}); | ||
}; | ||
|
||
keyDownHandler = (event: KeyboardEvent): void => { | ||
const { defaultPrevented, key } = event; | ||
|
||
|
@@ -867,9 +859,8 @@ export class InputTimePicker | |
this.popoverEl = el; | ||
}; | ||
|
||
private setInputAndTransitionEl = (el: HTMLCalciteInputElement): void => { | ||
private setInputEl = (el: HTMLCalciteInputElement): void => { | ||
this.calciteInputEl = el; | ||
this.transitionEl = el; | ||
}; | ||
|
||
private setCalciteTimePickerEl = (el: HTMLCalciteTimePickerElement): void => { | ||
|
@@ -978,9 +969,6 @@ export class InputTimePicker | |
async componentWillLoad(): Promise<void> { | ||
setUpLoadableComponent(this); | ||
await Promise.all([setUpMessages(this), this.loadDateTimeLocaleData()]); | ||
if (this.open) { | ||
onToggleOpenCloseComponent(this); | ||
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. With this removed, can the popover be open by default? |
||
} | ||
} | ||
|
||
componentDidLoad() { | ||
|
@@ -996,6 +984,8 @@ export class InputTimePicker | |
}), | ||
); | ||
} | ||
|
||
this.openHandler(); | ||
} | ||
|
||
disconnectedCallback() { | ||
|
@@ -1034,7 +1024,7 @@ export class InputTimePicker | |
onCalciteInputTextInput={this.calciteInternalInputInputHandler} | ||
onCalciteInternalInputTextFocus={this.calciteInternalInputFocusHandler} | ||
readOnly={readOnly} | ||
ref={this.setInputAndTransitionEl} | ||
ref={this.setInputEl} | ||
role="combobox" | ||
scale={this.scale} | ||
status={this.status} | ||
|
@@ -1046,9 +1036,10 @@ export class InputTimePicker | |
id={dialogId} | ||
label={messages.chooseTime} | ||
lang={this.effectiveLocale} | ||
onCalcitePopoverBeforeClose={this.popoverBeforeCloseHandler} | ||
onCalcitePopoverBeforeOpen={this.popoverBeforeOpenHandler} | ||
onCalcitePopoverClose={this.popoverCloseHandler} | ||
onCalcitePopoverOpen={this.popoverOpenHandler} | ||
open={this.open} | ||
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. With this removed, can the popover be open by default? |
||
overlayPositioning={this.overlayPositioning} | ||
placement={this.placement} | ||
ref={this.setCalcitePopoverEl} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.