Skip to content

Make checkboxes not use interior mutability#2976

Merged
Keavon merged 3 commits intomasterfrom
make-checkboxes-not-use-interior-mutability
Aug 3, 2025
Merged

Make checkboxes not use interior mutability#2976
Keavon merged 3 commits intomasterfrom
make-checkboxes-not-use-interior-mutability

Conversation

@timon-schelling
Copy link
Copy Markdown
Member

Closes #2974

@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 2, 2025 17:06 Inactive
@timon-schelling
Copy link
Copy Markdown
Member Author

timon-schelling commented Aug 2, 2025

image image

tested grid overlay checkbox in desktop

@TrueDoctor TrueDoctor requested a review from Keavon August 2, 2025 18:37
@Keavon Keavon marked this pull request as draft August 2, 2025 20:03
Copy link
Copy Markdown
Member

@Keavon Keavon left a comment

Choose a reason for hiding this comment

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

This entirely breaks the functionality.

@timon-schelling timon-schelling force-pushed the make-checkboxes-not-use-interior-mutability branch from a032521 to d9d1164 Compare August 3, 2025 13:38
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 13:41 Inactive
@timon-schelling timon-schelling force-pushed the make-checkboxes-not-use-interior-mutability branch from d9d1164 to e81776d Compare August 3, 2025 13:46
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 13:51 Inactive
@timon-schelling timon-schelling force-pushed the make-checkboxes-not-use-interior-mutability branch from e81776d to a478d83 Compare August 3, 2025 14:02
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 14:05 Inactive
@timon-schelling
Copy link
Copy Markdown
Member Author

@Keavon, Sorry I misunderstood what the interior mutability of CheckboxId was used for and tested the wrong thing. I think I tested the correct thing now, clicking a label now toggles the associated Checkbox. Serialization of CheckboxId is still working as expected, this PR still makes Chechboxes work on desktop.

2025-08-03.14-00-19.mp4

@timon-schelling timon-schelling marked this pull request as ready for review August 3, 2025 14:13
@timon-schelling timon-schelling requested a review from Keavon August 3, 2025 14:13
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 14:32 Inactive
@Keavon Keavon force-pushed the make-checkboxes-not-use-interior-mutability branch from ad46341 to dd6f28a Compare August 3, 2025 19:20
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 19:24 Inactive
@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 3, 2025 19:34 Inactive
@Keavon Keavon merged commit 8fad295 into master Aug 3, 2025
4 checks passed
@Keavon Keavon deleted the make-checkboxes-not-use-interior-mutability branch August 3, 2025 22: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.

Make checkboxes not use interior mutability

2 participants