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.

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

Make settings reset button only reset current level #3855

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

franknoirot
Copy link
Collaborator

Closes #2435, updates an E2E test mentioned in 2435 to expect this new scoped behavior.

Copy link

qa-wolf bot commented Sep 10, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Sep 10, 2024 6:00pm

// We instead pass in the default directory since it's asynchronous
// to re-initialize, and that can be done by the caller.
newSettings.app.projectDirectory.default = event.defaultDirectory
if (!('level' in event)) return {}
Copy link
Contributor

Choose a reason for hiding this comment

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

That's a neat one. Why not if (!event.level) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's the same in this case! I could totally do that, and it's probably a hair better.

In other type narrowing for these XState events we'll have data objects and have to narrow based on the keys within the data object, that may or may not exist. TS will yell at you if you try to type narrow in that instance with something like (!event.data?.innerKey), so you have to narrow with something like (!('data' in event && event.data.innerKey)). So I've just matched that idiom.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I understand all data in XState (5) is strongly typed, so I'd be surprised if we ever need in but please ping me so I can collect some examples for the ol' brain vault :D

@franknoirot franknoirot merged commit 292f898 into main Sep 11, 2024
24 checks passed
@franknoirot franknoirot deleted the franknoirot/2435/scope-settings-reset branch September 11, 2024 13:39
@franknoirot franknoirot mentioned this pull request Sep 13, 2024
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.

reset settings resets all settings versus scoped to the pane
2 participants