Remove view input properties from StateObject - #24099
Merged
Merged
Conversation
Collaborator
Generated by 🚫 Danger |
Contributor
|
| App Name | WordPress Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr24099-010a901 | |
| Version | 25.7.1 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 010a901 | |
| App Center Build | WPiOS - One-Offs #11554 |
Contributor
|
| App Name | Jetpack Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr24099-010a901 | |
| Version | 25.7.1 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 010a901 | |
| App Center Build | jetpack-installable-builds #10585 |
kean
reviewed
Feb 20, 2025
| self.pluginInfo = .init(name: plugin.name, author: plugin.author, shortDescription: plugin.author) | ||
| self.service = service | ||
| _viewModel = StateObject(wrappedValue: .init(slug: slug, service: service)) | ||
| _viewModel = StateObject(wrappedValue: .init(service: service)) |
Contributor
There was a problem hiding this comment.
Hey, just to confirm, the way it works now is that the ViewModel is stateless and can be re-used for different plugins, correct?
I typically use id to ensure the view is re-created when a new object is displayed, but that is also a common approach.
Contributor
Author
There was a problem hiding this comment.
the ViewModel is stateless
Kind of. It stores fetched data, so it has some state. But it does not aware of view's state (like via the old slug property).
I typically use id to ensure the view is re-created
Do you mean using id in the PluginDetailsView, or in its call sites? If you mean call sites, I think that's implied at the moment because PluginDetailsView is used in ForEach?
kean
self-requested a review
February 26, 2025 20:27
kean
approved these changes
Feb 26, 2025
crazytonyli
force-pushed
the
self-hosted-site-plugin-management-plugins-sections
branch
from
February 27, 2025 10:22
1ef825d to
bcf404e
Compare
Base automatically changed from
self-hosted-site-plugin-management-plugins-sections
to
trunk
February 27, 2025 11:23
crazytonyli
force-pushed
the
self-hosted-site-plugin-management-refine-ui
branch
from
February 27, 2025 19:44
ee1205e to
010a901
Compare
crazytonyli
enabled auto-merge
February 27, 2025 19:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


This is a follow-up PR for #24072 (comment).
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txtif necessary.Testing checklist: