-
-
Notifications
You must be signed in to change notification settings - Fork 664
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
Feat: new projects list #6873
Feat: new projects list #6873
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
c665cd2
to
60320dd
Compare
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
ace7513
to
b8c1008
Compare
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
b8c1008
to
d4f47fc
Compare
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
@@ -139,6 +149,7 @@ export const ProjectListNew = () => { | |||
); | |||
|
|||
const showProjectFilterButtons = useUiFlag('projectListFilterMyProjects'); | |||
const projectsListNewCards = useUiFlag('projectsListNewCards'); |
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.
❌ Getting worse: Complex Method
ProjectListNew increases in cyclomatic complexity from 21 to 23, threshold = 10
padding: theme.spacing(1, 2), | ||
borderTop: `1px solid ${theme.palette.grey[300]}`, | ||
backgroundColor: theme.palette.grey[100], | ||
boxShadow: 'inset 0px 2px 4px rgba(32, 32, 33, 0.05)', // FIXME: replace with variable |
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.
don't we have variable for this yet?
@@ -139,6 +149,7 @@ export const ProjectListNew = () => { | |||
); | |||
|
|||
const showProjectFilterButtons = useUiFlag('projectListFilterMyProjects'); | |||
const projectsListNewCards = true; |
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 probably meant to make this depend on the UI flag, right?
frontend/src/component/project/ProjectCard/ProjectCardIcon/ProjectCardIcon.tsx
Outdated
Show resolved
Hide resolved
frontend/src/component/project/ProjectCard/ProjectCardIcon/ProjectCardIcon.tsx
Outdated
Show resolved
Hide resolved
borderTop: `1px solid ${theme.palette.grey[300]}`, | ||
backgroundColor: theme.palette.grey[100], |
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.
How does this work in dark mode? Feels kinda weird to use the palette directly instead of color names?
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.
This is a placeholder for now.
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.
I'm assuming that the Legacy...
stuff is just the old cards renamed?
Can we instead consider naming the new stuff New...
? That serves two purposes:
- PR size: It makes the set of changes smaller. It doesn't look like you've added an extra 250 lines of code.
- Consistency: We have no other components in the front end called
Legacy...
. However, we do have a number of components calledNew...
. I think we should stick to that pattern.
What do you think?
display: '-webkit-box', | ||
boxOrient: 'vertical', | ||
textOverflow: 'ellipsis', | ||
overflow: 'hidden', | ||
alignItems: 'flex-start', | ||
WebkitBoxOrient: 'vertical', |
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.
This is deprecated (according to MDN). Can we use a different property?
<StyledDivHeader data-loading> | ||
<ProjectCardIcon mode={mode} /> | ||
<StyledBox> | ||
<StyledH2Title>{name}</StyledH2Title> |
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.
Should be h3
<StyledParagraphInfo data-loading> | ||
{featureCount} | ||
</StyledParagraphInfo> | ||
<p data-loading>toggles</p> |
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.
<p data-loading>toggles</p> | |
<p data-loading>Flags</p> |
<StyledParagraphInfo data-loading> | ||
{health}% | ||
</StyledParagraphInfo> | ||
<p data-loading>health</p> |
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.
Should be one paragraph. The number can be in a span if it needs to be styled differently.
…jectCardIcon.tsx Co-authored-by: Thomas Heartman <thomas@getunleash.io>
…jectCardIcon.tsx Co-authored-by: Thomas Heartman <thomas@getunleash.io>
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
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.
❌ Code Health Quality Gates: FAILED
- Declining Code Health: 1 findings(s) 🚩
TODO: test 0 members project |
About the changes
New card view for list of projects.
![image](https://private-user-images.githubusercontent.com/2625371/323510749-14dffed1-cc4f-41e5-859a-80773ceb4e7f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjAwNjg2ODUsIm5iZiI6MTcyMDA2ODM4NSwicGF0aCI6Ii8yNjI1MzcxLzMyMzUxMDc0OS0xNGRmZmVkMS1jYzRmLTQxZTUtODU5YS04MDc3M2NlYjRlN2YucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDcwNCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA3MDRUMDQ0NjI1WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MmZlYTg3NTg5MTVmZjIyMTk0ZDBlZDlmZWRkODNhMDViM2YzMmViYTQ5OTk5NmFlY2M1NTllZGY2NjkzZDkzYyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.8yJTDDxGl5iOtXWkn9UkgY18lFpHGv9ubVyQMjOWG9c)