Skip to content

Conversation

chintankavathia
Copy link
Member

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@chintankavathia chintankavathia marked this pull request as ready for review September 17, 2025 10:39
@chintankavathia chintankavathia requested a review from a team as a code owner September 17, 2025 10:39
@chintankavathia chintankavathia force-pushed the refactor/row-wrapper/signals branch 2 times, most recently from 4cba1ad to 2b862a9 Compare September 29, 2025 05:57
readonly ariaGroupHeaderCheckboxMessage = input.required<string>();

context!: RowDetailContext<TRow> | GroupContext<TRow>;
readonly context = linkedSignal<RowDetailContext<TRow> | GroupContext<TRow>>(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I guess this can a computed

Copy link
Member Author

@chintankavathia chintankavathia Oct 3, 2025

Choose a reason for hiding this comment

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

As I mentioned here #347 (comment)
computed won't trigger with markForCheck()

Comment on lines -144 to 146
if (this.rowDiffer.diff(this.row)) {
if ('group' in this.context) {
this.context.group = this.row as Group<TRow>;
const row = this.row();
if (this.rowDiffer.diff(row)) {
if ('group' in this.context()) {
this.context.set({
group: row as Group<TRow>,
expanded: this.expanded(),
rowIndex: this.rowIndex()
});
} else {
this.context.row = this.row as TRow;
this.context.set({
row: row as TRow,
expanded: this.expanded(),
rowIndex: this.rowIndex(),
disabled: this.disabled()
});
}
this.cd.markForCheck();
}
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just do:

if (this.rowDiffer.diff(this.row)) {
   this.cd.markForCheck();
}

@chintankavathia chintankavathia force-pushed the refactor/row-wrapper/signals branch from 2b862a9 to de24043 Compare October 3, 2025 04:41
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