fix: use option property to ensure options is treated as state #9953
Conversation
This file contains 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
deboer-tim
requested review from
cdrage and
SoniaSandler
and removed request for
a team
|
Some tests are failing |
See podman-desktop#9917 for background. Svelte reuses component instances whenever possible. The problem here is that the Dropdown options are being set via child option elements, created via buildOptions. When you switch pages all of the explicitly set properties and state are applied, but since options is not set as a property, nor state, it is not applied. The simplest fix in this case is using the options parameter directly, which we do in other cases and causes Svelte to correctly treat it as state. This is only avoiding the problem though, so I've opened podman-desktop#9952 for future follow up. I've checked and we have no other child options where this would currently happen, but the real fix is possibly a Svelte fix, removing child option support, or finding a fix directly in Dropdown. Fixes podman-desktop#9917. Signed-off-by: Tim deBoer <git@tdeboer.ca>
The test was bad - checking the text content of a select or dropdown _always_ passes when the value you're looking for is also a child option. Switching to not use children exposes the invalid test. This just removes the invalid line, which reverts this test to exactly where it was before we added Dropdown, just checking that the correct component is used. The correct initial selection behaviour is checked in the following test. Signed-off-by: Tim deBoer <git@tdeboer.ca>
deboer-tim
force-pushed
the
dropdown-options
branch
from
d4d9dac to
5fe138e
Compare
cdrage
approved these changes
Dec 16, 2024
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.
What does this PR do?
See #9917 for background. Svelte reuses component instances whenever possible. The problem here is that the Dropdown options are being set via child option elements, created via buildOptions. When you switch pages all of the explicitly set properties and state are applied, but since options is not set as a property, nor state, it is not applied.
The simplest fix in this case is using the options parameter directly, which we do in other cases and causes Svelte to correctly treat it as state.
This is only avoiding the problem though, so I've opened #9952 for future follow up. I've checked and we have no other child options where this would currently happen, but the real fix is possibly a Svelte fix, removing child option support, or finding a fix directly in Dropdown.
Screenshot / video of UI
N/A
What issues does this PR fix or reference?
Fixes #9917.
How to test this PR?
See #9917.