Skip to content

Conversation

@Caele
Copy link
Collaborator

@Caele Caele commented Apr 4, 2025

Motivation

Emitter needs to be defined even on sheets to exits for charts that use it, even if it isn't exposed through the Sheet api.

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 requested a review from Copilot April 7, 2025 06:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@Caele Caele requested a review from a team April 7, 2025 06:46
@Caele Caele merged commit 9e8f7cf into main Apr 7, 2025
10 checks passed
@Caele Caele deleted the tsm/sheet-emitter branch April 7, 2025 08:27
style.padding = '4px';
}

const emitter = new EventEmitter();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this maybe will create a new emitter for all cells when any cell is added or removed from the sheet

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea probably, I'll take another look

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