-
Notifications
You must be signed in to change notification settings - Fork 532
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
Setup tests in presence tracker #16040
Setup tests in presence tracker #16040
Conversation
4007cdd
to
12e2117
Compare
00488be
to
2d3695b
Compare
2d3695b
to
45e57e0
Compare
0592643
to
47e6b49
Compare
47e6b49
to
720b6e8
Compare
experimental/framework/data-objects/src/signaler/test/signaler.spec.ts
Outdated
Show resolved
Hide resolved
5af0e57
to
fd92879
Compare
fd92879
to
d5c2a0e
Compare
149003e
to
cb4b43f
Compare
cb4b43f
to
e7f5356
Compare
e7f5356
to
b4af36a
Compare
b4af36a
to
f37a7c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit surprised to see the change to make these DataObjects -- I think the tests look fine, but not sure we want to require persisting data for these scenarios?
name: string; | ||
} | ||
|
||
export class FocusTracker extends DataObject implements IFocusTracker { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choosing to make FocusTracker a DataObject means it becomes part of the persisted state of the container - I'm not sure if this is something we want? At minimum it is a pretty significant change in the purpose of the demo.
closing in favor of #16274 |
AB#1055