Closed iam-flo closed 1 month ago
We also have colors defined for border and card that you can make use off
Rework and ready to merge
It looks like this in the Storybook now: https://66a8981a27ced8fef3190d41-wvdgwzpzof.chromatic.com/?path=/docs/ui-appissuecard--docs
Some points that I noticed, see screenshot:
App
prefix, this is only important for the selectorIssueCard
is a component building on the blocks in the UI/
directory, our design system, the component should go under Components/Core/IssueCard
for now. pullRequest
-> issue
url
but you are not using it, I'd expect that you can click Artemis #12
and it opens to the issue linkOPEN
to CLOSED
nothing changes, it should change to the correct icon with the correct color, open does also not use the correct colorrepository.nameWithOwner
, repository.defaultBranch
, repository.visibility
, repository.url
in my opinion, just the name, then you also don't need to nest itcreatedAt
could be a dayjs
date or so that is correctly formatted in the UIfileDiff
icon but it is not in the github red for me, maybe there are some Storybook issues? Also I don't know how to configure it in the propsbug
and enhancement
. I also can't see them in the input control objectThis is currently the component API:
<app-issue-card
[pullRequest]="{title: 'Add feature X', number: 12, deletions: 5, url: 'http://example.com', state: 'CLOSED', repository: {name: 'Artemis', nameWithOwner: 'artemis-education/artemis', defaultBranch: 'master', visibility: 'PUBLIC', url: 'http://example.com'}, createdAt: 'Jan 1', pullRequestLabels: {'_constructor-name_':'Set'}, reviews: {'_constructor-name_':'Set'}}"
></app-issue-card>
Maybe we can not pass a complex type there, does it make sense, maybe not? Think like SwiftUI previews, this is basically what Storybook is for us
@GODrums I added a hover effect for the component, so that it is clearly clickable. I. think I implemented all requested changes. If you want to support, you can take a look at making the labels look like from GitHub. Haven't been able to implement that. In the future please use rebase for PRs as this cleans up the git tree. I will squash the commits before merge to clean everything up.
@iam-flo Sorry, I was out of line committing in your PR without asking. I think I also forgot to git pull
before merging since I was doing it from mobile.
Our usual convention is that @FelixTJDietrich merges all PRs with squashing, so no need to worry about git trees.
I think there are few smaller QoL things of the feedback left, I'll have a quick look and mark them.
@GODrums thank you for the suggestions 😄
LGTM, Thanks :)
Motivation
Adds a pull request widget component and the necessary data to display pull requests of a user
Description
Screenshots (if applicable)
Checklist
General
Client (if applicable)
Server (if applicable)