-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Angular: Add random attribute to bootstrapped selector #24972
Angular: Add random attribute to bootstrapped selector #24972
Conversation
Quick question: Will this change impact snapshot testing? Given that the id is always going to be inconsistent |
I will check this. |
I am not too familiar with how the snapshot testing works, but possibly. The reason I went with One reason I didn't try to only apply it to the docs, is to avoid this error from any way the renderer is used. Maybe an incremental counter could be used, instead of a random value. It may be predictable enough and avoid a dependency. |
I think that applying the logic to the DocsRenderer should be sufficient to fix the bug and circumvent any possible snapshot-related issues in the normal Story view. Let me know, whether you want to change it. I can also take care of it if you want :) |
@valentinpalkovic I can probably change it tomorrow, unless you want to go ahead and take care of it. |
I changed the implementation. It now only affects |
Closes #23875
What I did
Each story needs a unique selector to bootstrap the Angular app. #15501 had appended a unique id to the
storyId
, but when theprepareForInline
function was removed, that part of it must not have been moved to a new place. I went with a similar idea, but I did it a little different. Using the samenanoid
dependency, I added an attribute that is set to thestoryId
with a unique string appended. This attribute is then added to the element that will be bootstrapped as an attribute. The boostrap selector changes fromstoryId
tostoryId[uniqueAttribute]
.I did this in the
AbstractRenderer
, so it applies to all stories, but I think it is only needed by theDocsRenderer
. If we don't want theCanvasRenderer
to use the attribute in it's selector then I can move that attribute initialization to theDocsRenderer
.I tried to preserve the
storyId
, in case something is using it, but I didn't think about it causing twoid
DOM attributes to be the same. If that should not happen then I can try to find a spot to adjust the id, like theprepareForInline
function used to do, but I didn't see a clear equivalent.I also added a test for this, since there didn't seem to be one.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
yarn task --task sandbox --start-from auto --template angular-cli/default-ts
<Story of={ButtonStories.Primary}/><Story of={ButtonStories.Primary}/>
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
bug
: Internal changes that fixes incorrect behavior.maintenance
: User-facing maintenance tasks.dependencies
: Upgrading (sometimes downgrading) dependencies.build
: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup
: Minor cleanup style change. Will not show up in release changelog.documentation
: Documentation only changes. Will not show up in release changelog.feature request
: Introducing a new feature.BREAKING CHANGE
: Changes that break compatibility in some way with current major version.other
: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>