Ensure events are emitted on user-interaction and not programmatically

[jcfranco]

Description

This is a continuation of #2033. Non-change/input events are listed here.

---

Actual Behavior

Some components emit events regardless of internal or external (user) modifications.

Expected Behavior

Public events should emit from user-interaction and not when props are modified programmatically.

Relevant Info

Component/Event candidates

• action-bar.tsx fix(action-pad, action-bar): toggleChange event emit only on user interaction #4275 (@anveshmekala )
  • 66 this.calciteActionBarToggle.emit();
• action-pad.tsx fix(action-pad, action-bar): toggleChange event emit only on user interaction #4275 (@anveshmekala)
  • 52 this.calciteActionPadToggle.emit();
• [ ] combobox.tsx (implements openClose)
  • [ ] 462 this.active ? this.calciteComboboxOpen.emit() : this.calciteComboboxClose.emit();
• dropdown.tsx
  • 314 this.calciteDropdownSelect.emit(); - – already emits on user interaction only @driskull
  • [ ] 386 this.active ? this.calciteDropdownOpen.emit() : this.calciteDropdownClose.emit(); (implements openClose)
• inline-editable.tsx
  • 280 this.calciteInlineEditableEditCancel.emit(); – already emits on user interaction only @jcfranco
• [ ] input-date-picker.tsx (implements openClose)
  • [ ] 530 ? this.calciteInputDatePickerOpen.emit()
  • [ ] 531 : this.calciteInputDatePickerClose.emit();
• [ ] modal.tsx (implements openClose)
  • [ ] 333 this.active ? this.calciteModalOpen.emit() : this.calciteModalClose.emit();
• [ ] popover.tsx @driskull @jcfranco (implements openClose)
  • 388 this.open ? this.calcitePopoverOpen.emit() : this.calcitePopoverClose.emit();

Things to look out for

• Events emitted in a prop watcher may need to be moved elsewhere to avoid firing when props are programmatically modified
• Internal events can be refactored in a later milestone, the priority is for public ones
• Deprecated events (current or new ones) will not be modified

Related issues

#3453
#3191
#2033

Proposed Advantages

Consistent eventing will help determine its context.

Which Component

See list from description. We'll update the list accordingly if there are some we missed.

Relevant Info

No response

Exceptions

Events related to animations are excepted from this issue, since these kinds of events cannot be directly controlled by properties.

[Elijbet]

An issue I am working with (#5326) requires a bit of clarification on this.
Looks like components that have animations on them, such as the ones with openClose interface, need to be an exception to this, as there's no way to directly control when an animation completes by setting a property. And so we would emit also on the initial load of these components, not triggered by user interaction.

A follow-up issue is that if we decided that we now do emit on components that have animation on them, do we emit on initial load if these have duration set to 0?

I think it should emit on the initial load if it is a component with animation that has the animation-duration set to 0 so that it is consistent with instances where there is an animation on the initial load.

Maybe @eriklharper could comment on this more, as he brought up that this initial load animation event concept is similar to componentOnReady or other Stencil lifecycle methods that fire on initial load to communicate certain states of the component's initial load.

[driskull]

A follow-up issue is that if we decided that we now do emit on components that have animation on them, do we emit on initial load if these have duration set to 0?

I would say no. We should only emit on user interaction.

I think it should emit on the initial load if it is a component with animation that has the animation-duration set to 0 so that it is consistent with instances where there is an animation on the initial load.

If we did this it would be inconsistent with how our other events operate.

[eriklharper]

I think it should emit on the initial load if it is a component with animation that has the animation-duration set to 0 so that it is consistent with instances where there is an animation on the initial load.

If we did this it would be inconsistent with how our other events operate.

How will a developer know that a component is no longer animating on initial load?

[driskull]

How will a developer know that a component is no longer animating on initial load?

Yeah that's a fair point. I'm ok with going with events on initial load then. It seems thats what Franco communicated so lets go with that.

[eriklharper]

Cool, yeah when Eliza asked me this question, I realized that the main events we're concerned about are events emitted as a result of direct prop manipulation, but not necessarily all events that aren't directly tied to user interaction, like animation start and end events. Because animation events can't be directly controlled via props, they are kinda an exception to this concept.

[driskull]

Yeah that makes sense.

[anveshmekala]

Cool, yeah when Eliza asked me this question, I realized that the main events we're concerned about are events emitted as a result of direct prop manipulation, but not necessarily all events that aren't directly tied to user interaction, like animation start and end events. Because animation events can't be directly controlled via props, they are kinda an exception to this concept.

i second this. Because we don't provide props to control the animation of a given component and for some the animation is based on user interaction and other's its default, its better if we can except openClose events from this pattern.

[geospatialem]

Spike effort to determine next steps and applicability.

[jcfranco]

I agree with the above discussion regarding OpenCloseComponents being an exception to this rule. #7613 will add more info regarding this to our conventions.

Based on ☝️, open/close components have been excluded from the list. The remaining components already emit only on user-interaction.

[github-actions]

cc @geospatialem, @brittneytewks

[jcfranco]

Closing since all listed components emit as expected.