-
Notifications
You must be signed in to change notification settings - Fork 61
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 code from mobile view of setting bounty status as open if github issue is open #1164
Conversation
looks good just added one comment cause we should rename this file if we arent using the github user assigned stuff anymore |
@kevkevinpal is this fine |
<Pill isOpen={isOpen}> | ||
<div>{isOpen ? 'Open' : 'Closed'}</div> | ||
</Pill> | ||
<Pill isOpen={isOpen}>{/* <div>{isOpen ? 'Open' : 'Closed'}</div> */}</Pill> |
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.
we can delete this pill since there is no children to the component
@@ -55,7 +55,7 @@ const W = styled.div` | |||
display: flex; | |||
align-items: center; | |||
`; | |||
export default function GithubStatusPill(props: GithubStatusPillProps) { | |||
export default function StatusPill(props: StatusPillProps) { | |||
const { status, assignee, style } = props; |
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.
We can delete the status prop since that is no longer being used
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.
Done
@gouravmpk Based on your video, it looks like the bounty is missing the status of: open/assigned/closed. Is that right? This is what your video shows: This is what it should show and is currently live in production: We shouldn't be removing the status of the bounty in mobile you. All we're doing is removing the code that auto-sets the status of the bounty as "open" if the github issue is still open. |
Hey @ecurrencyhodler The status of the bounty is coming from GitHub issue only there is no other code or rather a property which sets bounty status except GitHub status ,this is my observation is that correct @kevkevinpal |
Interesting. Okay if that's the case we can merge this and handle the actual status of the bounty in |
Yep it would make sense. |
ya @ecurrencyhodler the status on here is for the github issue and the status of the bounty itself should be separate issues, I the scope of this PR is good as we're just removing the github status. Will review in a bit |
// interface PillProps { | ||
// readonly isOpen: boolean; | ||
// } | ||
// const Pill = styled.div<PillProps>` | ||
// display: flex; | ||
// justify-content: center; | ||
// align-items: center; | ||
// font-size: 12px; | ||
// font-weight: 300; | ||
// background: ${(p: any) => (p.isOpen ? '#49C998' : '#8256D0')}; | ||
// border-radius: 30px; | ||
// border: 1px solid transparent; | ||
// text-transform: capitalize; | ||
// padding: 12px 5px; | ||
// // padding:8px; | ||
// font-size: 12px; | ||
// font-weight: 500; | ||
// line-height: 20px; | ||
// white-space: nowrap; | ||
// border-radius: 2em; | ||
// height: 26px; | ||
// color: #fff; | ||
// margin-right: 10px; | ||
// width: 58px; | ||
// height: 22px; | ||
// left: 19px; | ||
// top: 171px; | ||
|
||
/* Primary Green */ | ||
// /* Primary Green */ | ||
|
||
border-radius: 2px; | ||
`; | ||
// border-radius: 2px; | ||
// `; |
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.
you can delete this if it is not being used
I think it looks good if you can remove the commented code |
Okay I am on it . |
@kevkevinpal I think this is good to go please check |
looks good to me nice work! |
Describe your changes
Edited StatusPill
Issue ticket number and link
#1143
Checklist before requesting a review
Video
open.bounty.remove.from.mobal.view.mp4
Note:
I'm not entirely certain if this aligns with the request, but in this pull request, I removed the code in StatusPill that typically sets the bounty status based on GitHub issues. This code previously determined whether the bounty status was 'Open' or 'Closed' based on the GitHub issues status. However, I observed that there is no other property or variable where the status of the bounty is being set. Could you please confirm if this is the intended change?
@kevkevinpal @ecurrencyhodler