-
Notifications
You must be signed in to change notification settings - Fork 121
make the deployment card as collapsible #3320
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
base: master
Are you sure you want to change the base?
Conversation
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.
Make sure to run npm run format and npm run typecheck.
Some notes before looking at the details of the implementation:
- Remove comments.
- If statements require braces around the
return; - Don't use shorthand terms like
eforevent - Single invocations don't require braces (lines 197-206)
- Record a video of how it looks like after your changes
|
Here’s a quick demo of the changes: Loom Video |
kresimir-coko
left a comment
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.
Good job, a couple things missing.
- Cursor should be cursor-pointer indicating that the card is clickable
- Clicks on the dropdown (as indicated in video) shouldn't open the card
- On line 92 we have a single line if statement, make sure the return is wrapped with braces
- On line 125 there is a leftover comment
Screen.Recording.2025-10-22.at.09.40.48.mov
|
@Yasir761 did you fix these 2 points?
|
Now they work as expected |
|
@Yasir761 Clicks on the dropdown (as indicated in video) shouldn't open the card. Make sure to check it manually if it works as expected, I pulled your code and clicked on the dropdown's options and it toggles the card |
|
@kresimir-coko, Hey, I just attached the Loom video showing that dropdown. Here is the link video |
|
@Yasir761 the one on the right, that opens when you click on the Button represented by the 3 vertical dots. Sorry for the confusion.
|
|
@kresimir-coko, can you review the changes? I guess I solved them. |

Description
Fix ProjectDeploymentListItem dropdown menu
Fixes #3266