Skip to content

+ add method eventWrapper() to eliminate all self=this and curry returned function for event listeners#5

Merged
ControlNet merged 17 commits intoControlNet:mainfrom
n0099:main
Nov 26, 2022
Merged

+ add method eventWrapper() to eliminate all self=this and curry returned function for event listeners#5
ControlNet merged 17 commits intoControlNet:mainfrom
n0099:main

Conversation

@n0099
Copy link
Contributor

@n0099 n0099 commented Nov 22, 2022

  • now listening more generic pointer events which including both touch and mouse events @ init
  • rename all event listeners from eventnameEvent to onEventName
    @ plot/br-heatmap.ts.BrHeatmap

…returned function for event listeners

* now listening more generic pointer events which including both touch and mouse events @ init
* rename all event listeners from `eventnameEvent` to `onEventName`
@ `plot/br-heatmap.ts`.BrHeatmap
@ControlNet ControlNet self-requested a review November 22, 2022 00:50
Copy link
Owner

@ControlNet ControlNet left a comment

Choose a reason for hiding this comment

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

Line 49: Please remove unnecessary console.log.

Please consider to edit /src/plot/line-chart-ts as well.

* redo what we did in prev commit 18358f2 for `plot/br-heatmap.ts`.BrLineChart
Copy link
Owner

@ControlNet ControlNet left a comment

Choose a reason for hiding this comment

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

Type checking error. Please revise the code.

@ControlNet
Copy link
Owner

Need revise the event listeners in line-chart.ts.

…Element` since its usage `this.svg.on()` is requiring `ValueFn<SVGSVGElement, unknown, void>` @ event listeners()

* pass the mousePos argu into `this.tooltip.update` @ onPointerMove()
* import missing types
@ `plot/line-chart.ts`.BrLineChart
@ControlNet ControlNet self-requested a review November 24, 2022 17:22
Copy link
Owner

@ControlNet ControlNet left a comment

Choose a reason for hiding this comment

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

image

It looks doesn't work on my end. Do you have any idea?

This reverts commit a281acf.
@n0099
Copy link
Contributor Author

n0099 commented Nov 25, 2022

It looks doesn't work on my end. Do you have any idea?

but webpack build will success without any ts error/warn

"typescript": "^4.2.2",

I suggest upgrade the ts version to 4.9

u've added the package.lock file into .gitignore in ffbe109, so we are using vary version of typescript and many other packages

@ControlNet ControlNet self-requested a review November 25, 2022 05:53
ControlNet and others added 7 commits November 25, 2022 17:07
…returned function for event listeners

* now listening more generic pointer events which including both touch and mouse events @ init
* rename all event listeners from `eventnameEvent` to `onEventName`
@ `plot/br-heatmap.ts`.BrHeatmap
* redo what we did in prev commit 18358f2 for `plot/br-heatmap.ts`.BrLineChart
…Element` since its usage `this.svg.on()` is requiring `ValueFn<SVGSVGElement, unknown, void>` @ event listeners()

* pass the mousePos argu into `this.tooltip.update` @ onPointerMove()
* import missing types
@ `plot/line-chart.ts`.BrLineChart
Copy link
Owner

@ControlNet ControlNet left a comment

Choose a reason for hiding this comment

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

Need revision.

@ControlNet ControlNet merged commit 4eb5768 into ControlNet:main Nov 26, 2022
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.

2 participants