Skip to content

Add support for draft issues to the StateLabel component - #1604

Merged
japf merged 6 commits into
mainfrom
japf/state-label-draft-issue
Nov 15, 2021
Merged

Add support for draft issues to the StateLabel component#1604
japf merged 6 commits into
mainfrom
japf/state-label-draft-issue

Conversation

@japf

@japfjapf commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

This support is adding support for draft issues to the <StateLabel> component.

Screenshots

  • Before

image

  • After

image

Merge checklist

  • Added/updated tests: looks like existing test does not have assertions at the icon level, so I didn't do any changes here.
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

@changeset-bot

changeset-botBot commented Nov 12, 2021•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e88a61d

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

This PR includes changesets to release 1 package
NameType
@primer/componentsPatch

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-actionsBot commented Nov 12, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js54.96 KB (+0.97% 🔺)
dist/browser.umd.js55.35 KB (+0.95% 🔺)

japf
japf commented Nov 12, 2021
View reviewed changes
Comment threadpackage.json
"license": "MIT",
"dependencies": {
"@primer/octicons-react": "^13.0.0",
"@primer/octicons-react": "^16.1.0",

@japfjapfNov 12, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The new icon I'm using here (IssueDraftIcon) was added here primer/octicons#614 and shipped with version 14. I'm not sure how we update dependencies in such cases.

@japfjapfNov 12, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is likely causing some test failures right now 😢

@rezrahrezrahNov 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like the snapshots are stale. Does running test:update command help the tests pass?

@japfjapfNov 12, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@rezrah looks like they are passing now with dd6b246

@japf
japf marked this pull request as ready for review November 12, 2021 14:54
@japf
japf requested review from a team and siddharthkpNovember 12, 2021 14:54
@japf

japf commented Nov 15, 2021

Copy link
Copy Markdown
ContributorAuthor

I just had a look at the changes in the package-lock.json and I'll need to fix that before we can merge this PR:
image

@japf
japf requested a review from rezrahNovember 15, 2021 09:36
@japf

japf commented Nov 15, 2021

Copy link
Copy Markdown
ContributorAuthor

I just had a look at the changes in the package-lock.json and I'll need to fix that before we can merge this PR: image

Fixed with a6e319f

The diff is looking good now:

image

@japf
japf enabled auto-merge (squash) November 15, 2021 15:55
@japf
japf merged commit d2090b5 into mainNov 15, 2021
@japf
japf deleted the japf/state-label-draft-issue branch November 15, 2021 15:59
This was referenced Nov 15, 2021
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siddharthkpsiddharthkpAwaiting requested review from siddharthkp
@rezrahrezrahAwaiting requested review from rezrah
1 more reviewer
@pksjcepksjcepksjce approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@japf@pksjce@rezrah