-
Notifications
You must be signed in to change notification settings - Fork 124
Update IncludeFragment to add data-nonce when a current nonce is present #3466
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
base: main
Are you sure you want to change the base?
Conversation
…c context to components. Update IncludeFragment to add data-nonce when a current nonce is present.
🦋 Changeset detectedLatest commit: 7e5c261 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Pull Request Overview
This PR introduces a new Primer::CurrentAttributes class that allows setting request-specific context (nonce) for components and updates the IncludeFragment component to conditionally add a data-nonce attribute when a current nonce is present.
- Introduces Primer::CurrentAttributes to hold request-specific data.
- Updates IncludeFragment initialization to optionally include a "data-nonce" attribute.
- Adds tests to verify rendering behavior for nonce, loading, src, and accept attributes.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
test/components/primer/alpha/include_fragment_test.rb | Adds tests to cover the new nonce behavior and loading. |
app/lib/primer/current_attributes.rb | Implements CurrentAttributes to hold request-specific nonce. |
app/components/primer/alpha/include_fragment.rb | Updates component initialization to use nonce and manage loading values. |
.changeset/sour-beds-draw.md | Documents the changes and version bump for release. |
Comments suppressed due to low confidence (1)
app/components/primer/alpha/include_fragment.rb:21
- [nitpick] Consider moving the assignment of @system_arguments[:loading] inside the 'if loading' block to avoid setting a nil value when loading is not provided, thereby improving clarity and reducing redundancy.
@system_arguments[:loading] = loading
be827d5
to
7e5c261
Compare
Authors: Please fill out this form carefully and completely.
Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.
What are you trying to accomplish?
I'm providing a new
Primer::CurrentAttributes
class that callers can use to set some request/context dependent information to be passed to components. The first use here is setting a uniquenonce
to allIncludeFragment
components without having to manually add it as a prop in every use-case.Screenshots
No UI changes.
Integration
This is totally optional.
List the issues that this change affects.
Part of #4942
Risk Assessment
What approach did you choose and why?
I'm using ActiveSupport::CurrentAttributes to create a new Primer specific class to hold data related to components. This class is request bound so it ensures we can share data across a single request.
Accessibility
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.