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.
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
Core: Introduce run over play in portable stories, and revert back play changes of 8.2 #28764
Core: Introduce run over play in portable stories, and revert back play changes of 8.2 #28764
Changes from 1 commit
339b541
653324d
d85f877
25ab52b
7f86411
bb34a32
e9c18ff
d7d9092
63d3aec
a53df3a
f9605a7
a762069
b1acd94
a6eea4b
fa39958
f139626
9d2af7c
89eddca
2772623
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 160 in code/core/src/preview-api/modules/store/csf/portable-stories.ts
src/preview-api/modules/store/csf/portable-stories.test.ts > composeStory > should compose with a play function
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.
style: Making
play
optional might cause issues if existing code relies on it being present.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.
style: Ensure
run
function provides the same functionality asplay
to avoid test failures.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.
style: Verify that
LoaderStory.run
correctly handles the data composition and spy assertions.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.
style: Confirm that
CSF3InputFieldFilled.run
correctly initializes the input field value.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.
style: Check if
CSF3InputFieldFilled.run
correctly interacts with the canvas element.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.
style: Ensure all stories in
testCases
are correctly rendered using therun
function.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.
style: Ensure
testingLibraryRender
is always configured to avoid runtime errors.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.
logic: Consider using a more descriptive variable name than
context.canvasElement
for clarity.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.
style: Ensure the
run
function is thoroughly tested to avoid any unforeseen issues.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.
style: Verify that the
run
function handles all scenarios previously managed byplay
.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.
check: Consider adding additional test cases to cover edge cases for the
run
function.Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF2Secondary story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF2StoryWithParamsAndDecorator story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders NewStory story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF3Primary story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF3Button story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF3ButtonWithRender story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders CSF3InputFieldFilled story
Check failure on line 175 in code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
src/__test__/composeStories/portable-stories.test.ts > Renders LoaderStory story
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.
style: Ensure that the new error handling for
testingLibraryRender
does not disrupt existing stories that might not have this configured.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.
style: Consider enabling the fallback to
svelteProjectAnnotations.renderToCanvas
for better backward compatibility.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.
style: Ensure that the
run
method is correctly implemented and behaves as expected in all scenarios.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.
style: Verify that the
run
method correctly replacesplay
and maintains the intended functionality.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.
style: Check for any potential side effects of renaming
play
torun
in the test cases.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.
style: Ensure
testingLibraryRender
is always configured to avoid runtime errors.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.
style: Consider enabling the fallback to
defaultProjectAnnotations.renderToCanvas
for better backward compatibility.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.
style: Using
container
instead ofbaseElement
might affect existing tests relying on the previous behavior.