- 
                Notifications
    
You must be signed in to change notification settings  - Fork 640
 
          Remove support for sx from FilteredActionList component
          #6876
        
          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
  
    Remove support for sx from FilteredActionList component
  
  #6876
              Conversation
          🦋 Changeset detectedLatest commit: b8fad14 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
 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  | 
    
| 
          
 👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!  | 
    
Updated FilteredActionList and FilteredActionListLoaders components to remove sx support.
| 
          
 👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/2844  | 
    
| 
          
 🟢 ci completed with status   | 
    
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 removes support for the sx prop from the FilteredActionList component as part of the migration away from styled-components. Since there is no current usage of sx on this component in dotcom, this change is safe to implement as a breaking change.
Key changes:
- Replace 
Boxcomponents with plaindivelements and corresponding CSS classes - Remove 
sxprop from component interface and implementation - Add CSS classes to maintain equivalent styling functionality
 
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description | 
|---|---|
| FilteredActionList.tsx | Remove sx prop from interface, replace BoxWithFallback with div, update ActionList styling | 
| FilteredActionList.module.css | Add ActionList CSS class with flex-grow styling | 
| FilteredActionListLoaders.tsx | Replace Box components with div elements and CSS classes | 
| FilteredActionListLoaders.module.css | Add LoadingSpinner and LoadingSkeletonContainer CSS classes | 
| .changeset/mighty-lizards-lick.md | Add changeset entry for major version bump | 
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.
Looks great! 🎉
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.
Shouldn't we be removing below as well?
const StyledHeader = styled.div`
  box-shadow: 0 1px 0 ${get('colors.border.default')};
  z-index: 1;
`
Closes #6789
Current
sxusage: 0✅ Don't need to separately update github-ui components to use @primer/styled-react
Changelog
Removed
Remove support for
sxfrom theFilteredActionListcomponent, and any associated stories, docs, and testsRollout strategy
This component currently has 0
sxusage in dotcom, and therefore does not need the @primer/styled-react wrapper. this will be confirmed again before merging.Merge checklist