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

feat: add useEmitter and corresponding viz api #1168

Merged
merged 4 commits into from
Mar 28, 2023
Merged

Conversation

Caele
Copy link
Collaborator

@Caele Caele commented Mar 21, 2023

Motivation

Adds new useEmitter hook to enable charts to affect mashups and client.

Requirements checklist

  • Api specification
    • Ran yarn spec
      • No changes OR API changes has been formally approved
  • Unit/Component test coverage
  • Correct PR title for the changes (fix, chore, feat)

When build and tests have passed:

  • Add code reviewers, for example @qlik-oss/nebula-core

@Caele Caele mentioned this pull request Mar 21, 2023
6 tasks
@Caele Caele marked this pull request as ready for review March 24, 2023 12:18
@Caele Caele requested review from veinfors, T-Wizard, niekvanstaveren and a team March 24, 2023 12:19
Copy link
Collaborator

@T-Wizard T-Wizard left a comment

Choose a reason for hiding this comment

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

consider if we want to expose a bit more then just on and off to the mashup
ex: once, removeAllListeners, setMaxListeners
but that could be done later when we see a need for it

apis/nucleus/src/viz.js Outdated Show resolved Hide resolved
@Caele
Copy link
Collaborator Author

Caele commented Mar 27, 2023

consider if we want to expose a bit more then just on and off to the mashup ex: once, removeAllListeners, setMaxListeners but that could be done later when we see a need for it

yea, keeping this experimental for now, so just adding bare minimum

@Caele Caele merged commit 034be90 into master Mar 28, 2023
@Caele Caele deleted the tsm/use-emitter branch March 28, 2023 16:16
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.

3 participants