Skip to content
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 ActionList #2405

Merged
merged 3 commits into from
Mar 28, 2023
Merged

Remove ActionList #2405

merged 3 commits into from
Mar 28, 2023

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Mar 28, 2023

What are you trying to accomplish?

This removes ActionList.

What approach did you choose and why?

These styles are an earlier version of the ActionList component. A newer version was added later to PVC. The styles will be added to dotcom for the places that still rely on this version.

What should reviewers focus on?

I removed also some Stories of other components that relied on the ActionList styles. Like NavList, Layout examples, TreeView. Is that ok? I assume the newer version is available in PVC's Lookbook.

Can these changes ship as is?

  • ⚠️ No! It's still needed on dotcom. 👉 PR.

@changeset-bot
Copy link

changeset-bot bot commented Mar 28, 2023

🦋 Changeset detected

Latest commit: 648bda4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/css Major

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

@github-actions github-actions bot temporarily deployed to Storybook Preview March 28, 2023 06:26 Inactive
@github-actions github-actions bot temporarily deployed to Storybook Preview March 28, 2023 06:29 Inactive
@simurai simurai marked this pull request as ready for review March 28, 2023 07:29
@simurai simurai requested a review from a team as a code owner March 28, 2023 07:29
Copy link
Contributor

@lukasoppermann lukasoppermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the code it looks great. 👍

I can not attest to the actual change, as I am not sure if/where this is used. But @langermank will probably be able to do that.

Copy link
Contributor

@langermank langermank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not goodbye, it's see you later (in dotcom) 🥲

@simurai
Copy link
Contributor Author

simurai commented Mar 28, 2023

@langermank Its not goodbye, it's see you later (in dotcom) 🥲

Yep, this version will live on for a bit longer in https://github.com/github/github/pull/266944

@simurai simurai merged commit 85f31cc into main Mar 28, 2023
@simurai simurai deleted the remove-actionlist branch March 28, 2023 23:25
@primer-css primer-css mentioned this pull request Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants