-
-
Notifications
You must be signed in to change notification settings - Fork 656
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: split projects view into "my projects" and "other projects" #6886
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: OK
- Improving 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: OK
- Improving 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: OK
- Improving Code Health: 1 findings(s) ✅
const projectsLists = useMemo(() => { | ||
if (!splitProjectList) { | ||
return { my: [], other: projects }; | ||
} | ||
|
||
const my: IProjectCard[] = []; | ||
const other: IProjectCard[] = []; | ||
|
||
for (const project of filteredProjects) { | ||
console.log('handling', project); | ||
if (shouldDisplayInMyProjects(myProjects)(project)) { | ||
my.push(project); | ||
} else { | ||
other.push(project); | ||
} | ||
} | ||
return { my, other }; | ||
}, [filteredProjects, myProjects, splitProjectList]); |
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 wanna extract this into its own function so that I can test it properly.
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: OK
- Improving 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: OK
- Improving 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: OK
- Improving 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: OK
- Improving Code Health: 1 findings(s) ✅
const ProjectGroup: React.FC<{ | ||
sectionTitle?: string; | ||
projects: IProjectCard[]; | ||
}> = ({ sectionTitle, projects }) => { | ||
return ( | ||
<StyledProjectGroupContainer> | ||
<ConditionallyRender | ||
condition={Boolean(sectionTitle)} | ||
show={ | ||
<Typography component='h3'>{sectionTitle}</Typography> | ||
} | ||
/> | ||
<ConditionallyRender | ||
condition={projects.length < 1 && !loading} | ||
show={ | ||
<ConditionallyRender | ||
condition={searchValue?.length > 0} | ||
show={ | ||
<TablePlaceholder> | ||
No projects found matching “ | ||
{searchValue} | ||
” | ||
</TablePlaceholder> | ||
} | ||
elseShow={ | ||
<TablePlaceholder> | ||
No projects available. | ||
</TablePlaceholder> | ||
} | ||
/> | ||
} | ||
elseShow={ | ||
<StyledItemsContainer> | ||
<ConditionallyRender | ||
condition={loading} | ||
show={() => | ||
loadingData.map((project: IProjectCard) => ( | ||
<ProjectCard | ||
data-loading | ||
onHover={() => {}} | ||
key={project.id} | ||
name={project.name} | ||
id={project.id} | ||
mode={project.mode} | ||
memberCount={2} | ||
health={95} | ||
featureCount={4} | ||
/> | ||
)) | ||
} | ||
elseShow={() => ( | ||
<> | ||
{projects.map( | ||
(project: IProjectCard) => ( | ||
<StyledCardLink | ||
key={project.id} | ||
to={`/projects/${project.id}`} | ||
> | ||
<ProjectCard | ||
onHover={() => | ||
handleHover( | ||
project.id, | ||
) | ||
} | ||
name={project.name} | ||
mode={project.mode} | ||
memberCount={ | ||
project.memberCount ?? | ||
0 | ||
} | ||
health={project.health} | ||
id={project.id} | ||
featureCount={ | ||
project.featureCount | ||
} | ||
isFavorite={ | ||
project.favorite | ||
} | ||
/> | ||
</StyledCardLink> | ||
), | ||
)} | ||
</> | ||
)} | ||
/> | ||
</StyledItemsContainer> | ||
} | ||
/> | ||
</StyledProjectGroupContainer> | ||
); | ||
}; |
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 just extracting the component that was already there, so that we can use it in two places. However, it feels pretty gnarly.
I'm unsure about moving it out into its own file, though, because then we'd have to add five extra parameters to the function:
loading
searchValue
CardComponent
ContainerComponent
handleHover
Still, it might be a neater approach. What do you think?
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.
Update: I went ahead with extracting this into its own file. I think it's the right thing to do.
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: OK
- Improving Code Health: 1 findings(s) ✅
const projectsListNewCards = useUiFlag('projectsListNewCards'); | ||
const filters = ['All projects', 'My projects']; | ||
const [filter, setFilter] = useState(filters[0]); | ||
const splitProjectList = useUiFlag('projectListFilterMyProjects'); |
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 better: Complex Method
ProjectListNew decreases in cyclomatic complexity from 23 to 17, threshold = 10
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: OK
- Improving Code Health: 1 findings(s) ✅
This PR removes the previous "my projects" filter in favor always splitting projects, but showing both on the main screen.
To make it a bit easier to work with, it also moves the project group component into its own file, causing some extra lines of code change. My apologies 🙇🏼