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.

Sign up for GitHub

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

Jump to bottom

Improve the "Node.isSomething" mixin pattern #1653

Closed
samreid opened this issue Sep 3, 2024 · 5 comments
Closed

Improve the "Node.isSomething" mixin pattern #1653

samreid opened this issue Sep 3, 2024 · 5 comments
Assignees
@zepumph
Labels
status:ready-for-review

Comments

@samreid
Copy link
Member

samreid commented Sep 3, 2024

Commit on the way for InteractiveHighlighting, we can do more like this for voicing, and reading block.
@zepumph
Copy link
Member

zepumph commented Sep 3, 2024

From work in phetsims/scenery-phet#869

samreid added a commit to phetsims/scenery-phet that referenced this issue Sep 3, 2024
@samreid
Add isInteractiveHighlighting, see phetsims/scenery#1653
67aa281
samreid added a commit to phetsims/axon that referenced this issue Sep 3, 2024
@samreid
Add isInteractiveHighlighting, see phetsims/scenery#1653
d480c06
samreid added a commit that referenced this issue Sep 3, 2024
@samreid
Add isInteractiveHighlighting, see #1653
aac48db
samreid added a commit to phetsims/scenery-phet that referenced this issue Sep 3, 2024
@samreid
Add isVoicing, see phetsims/scenery#1653 and #869
9c994d8
samreid added a commit that referenced this issue Sep 3, 2024
@samreid
Add isVoicing, see #1653 and phetsims/scenery-phet#869
70df130
zepumph added a commit to phetsims/friction that referenced this issue Sep 12, 2024
@zepumph
prefer "isMixin()" functions to the boolean getter (For Typescript), p…
11a9543 
…hetsims/scenery#1653

Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit that referenced this issue Sep 12, 2024
@zepumph
prefer "isMixin()" functions to the boolean getter (For Typescript), #…
f428b45 
…1653

Signed-off-by: Michael Kauzmann <[email protected]>
@zepumph
Copy link
Member

zepumph commented Sep 12, 2024

Ok. I committed the rest of these. So now we prefer these standalone functions instead of the boolean getter. I went one step further and added underscores to the getters.

@jessegreenberg can you please spot check this, while noting the new pattern and how it removes the need to type case things as the mixin types. Please also double check the one TODO I added.

@zepumph zepumph assigned jessegreenberg and unassigned zepumph Sep 12, 2024
@zepumph
Copy link
Member

zepumph commented Sep 12, 2024

A couple more coming in that I missed, one second.

zepumph added a commit that referenced this issue Sep 12, 2024
@zepumph
more isInteractiveHighlighting conversion (oops), #1653
d5da2a7 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/scenery-phet that referenced this issue Sep 12, 2024
@zepumph
more isInteractiveHighlighting conversion (oops), phetsims/scenery#1653
7da9fc0 
Signed-off-by: Michael Kauzmann <[email protected]>
@zepumph
Copy link
Member

zepumph commented Sep 12, 2024

ok done.

jessegreenberg referenced this issue Sep 13, 2024
@jessegreenberg
confirm todo, see phetsims/color-vision#159
d5075df
@jessegreenberg
Copy link
Contributor

I reviewed the commits, looks like a nice improvement to me and it matches the pattern used by layout traits.

A TODO was added for this issue, I confirmed it was correct and tested the behavior in quadrilateral.

Thanks!

@zepumph zepumph closed this as completed Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zepumph zepumph

Labels
status:ready-for-review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@samreid @jessegreenberg @zepumph