Skip to content
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

web: align the rest of events #887

Merged
merged 18 commits into from Jul 16, 2021
Merged

web: align the rest of events #887

merged 18 commits into from Jul 16, 2021

Conversation

eymar
Copy link
Collaborator

@eymar eymar commented Jul 14, 2021

No description provided.

@eymar eymar force-pushed the web/other_events_refactoring branch from 7f77904 to 969c97f Compare July 14, 2021 14:55
@eymar eymar changed the title web: update change and beforeInput event listeners web: align the rest of events Jul 16, 2021
Copy link
Collaborator

@Schahen Schahen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from notes I left I'm ok with this PR

thanks!

* search event is not supported in Firefox, so we skip it for now
* https://caniuse.com/mdn-api_htmlinputelement_search_event
*/
const val SEARCH = "search"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove this entirely

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


fun onInput(
options: Options = Options.DEFAULT,
listener: (SyntheticInputEvent<T, HTMLInputElement>) -> Unit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's rename T

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new name = "ValueType"
done

listener: (SyntheticMouseEvent) -> Unit
) : SyntheticEventListener<SyntheticMouseEvent>(event, options, listener) {
override fun handleEvent(event: Event) {
listener(SyntheticMouseEvent(event.unsafeCast<MouseEvent>()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's choose one of two - either we use unsafeCast or as

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsafeCast is now used in this and similar cases

nativeEvent: Event,
) : SyntheticEvent<EventTarget>(nativeEvent) {

val animationName: String = nativeEvent.asDynamic().animationName.unsafeCast<String>()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's have external interface instead of asDynamic casts

Copy link
Collaborator Author

@eymar eymar Jul 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

external interface AnimationEventDetails {
    val animationName: String
}

nativeEvent is of type Event, which is an external class, so our external interface can't inherit from the Event class.
So I added one more parameter to the constructor:

constructor(nativeEvent: Event, animationEventDetails: AnimationEventDetails)

and I use it like this:

SyntheticAnimationEvent(nativeEvent, nativeEvent.unsafeCast<AnimationEventDetails>())

the constructor is internal, so I think it can be like this, since it's our internal thing.

) : SyntheticEvent<EventTarget>(nativeEvent) {

val altKey: Boolean = nativeEvent.altKey
val changedTouched: TouchList = nativeEvent.changedTouches
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type: I guess it was supposed to be called changeTouches

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks!

val box = driver.findElement(By.id("box"))

TouchActions(driver)
.down(box.rect.x + 50, box.rect.y + 50)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may it we can just check sizes and click sort of in the middle (or wherever we want to click)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@eymar eymar marked this pull request as ready for review July 16, 2021 13:47
@eymar eymar merged commit 6d97c6d into master Jul 16, 2021
@eymar eymar deleted the web/other_events_refactoring branch July 16, 2021 15:56
mareklangiewicz pushed a commit to mareklangiewicz/compose-jb that referenced this pull request Feb 14, 2022
* web: update change and beforeInput event listeners

* web: update Clipboard events: copy, cut, paste

* web: update Keyboard events: keyup, keydown

* web: update select event (only for input and textarea)

* web: update FocusEvents: focus, blur, focusin, focusout

* web: update Touch Events

* web: remove search event listener as it's not supported in Firefox

* web: update Animation Events

* web: update scroll event

* web: rename WrappedEventListener to SyntheticEventListener

* refactor internal event listeners to avoid redundant lambdas

* remove WrappedEvent.kt

* move SyntheticInputEvent.kt to `events` package

* Add tests for Select element events

* remove import from another changelist

* web: add Form events: submit and reset

* fix PR discussions

* web: rename `attrsBuilder` to `attrs`

Co-authored-by: Oleksandr Karpovich <oleksandr.karpovich@jetbrains.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants