-
Notifications
You must be signed in to change notification settings - Fork 9
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
Playground instance previewed in edit view should save changes #2165
Conversation
Host Test Results 1 files ±0 1 suites ±0 23m 16s ⏱️ +35s Results for commit 1973853. ± Comparison against base commit 22ebcca. This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
if (!this.cardApiCache) { | ||
this.cardApiCache = await this.cardService.getAPI(); | ||
} |
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.
no need to cache this--it's actually cached in the loader, you can just get it each time that you need it.
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.
It's cached so that it can be reused in the unloadResource function. Otherwise, unloadResource would have to be an async function, but we need it to be synchronous for use in registerDestructor. Do you have any other idea how to get cardAPI in unreloadResource?
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.
ah! I see! make sense let's leave it as is
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.
this looks really good! exactly like we discussed
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 have tested this and I dont see any degression in auto save. LGTM
cardResource
in the playground panel an auto-saving card.