-
Notifications
You must be signed in to change notification settings - Fork 3
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
Create project card and some of the home screen #86
Closed
henrycatalinismith
wants to merge
11
commits into
zetkin:main
from
henrycatalinismith:issue-74-project-card
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3225e92
Install Storybook and set up stories for the MessageForm component
henrycatalinismith 99fb6e0
Create project card component and some of the home screen
henrycatalinismith 778e7f2
Improve project card alignment with design
henrycatalinismith ebdea32
Fix lint error
henrycatalinismith 5202b62
Force dynamic rendering on the home page
henrycatalinismith 1afa125
Fix linter errors
henrycatalinismith c1c7c38
Move devDependencies to root
henrycatalinismith 11a3eae
Upgrade to [email protected]
henrycatalinismith 696cf4b
Merge pull request #1 from henrycatalinismith/devDependencies
henrycatalinismith d80399e
Remove @storybook/addon-onboarding reference
henrycatalinismith 690e697
Rename ProjectsDashboard to HomeDashboard
henrycatalinismith File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
node_modules/ | ||
.idea/ | ||
config/ | ||
.yarn/ | ||
|
||
# macOS | ||
.DS_Store |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
nodeLinker: node-modules |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
"version": "0.0.1", | ||
"description": "Root package of Lyra", | ||
"private": true, | ||
"packageManager": "[email protected]", | ||
"engines": { | ||
"node": ">=16.0.0" | ||
}, | ||
|
@@ -12,15 +13,23 @@ | |
] | ||
}, | ||
"scripts": { | ||
"build": "yarn workspaces run build", | ||
"test": "yarn workspaces run test", | ||
"lint:formatting": "yarn workspaces run lint:formatting", | ||
"lint:eslint": "yarn workspaces run lint:eslint" | ||
"build": "yarn workspaces foreach -A run build", | ||
"test": "yarn workspaces foreach -A run test", | ||
"lint:formatting": "yarn workspaces foreach -A run lint:formatting", | ||
"lint:eslint": "yarn workspaces foreach -A run lint:eslint" | ||
}, | ||
"keywords": [ | ||
"lyra" | ||
], | ||
"devDependencies": { | ||
"@chromatic-com/storybook": "^1.3.5", | ||
"@storybook/addon-essentials": "^8.0.10", | ||
"@storybook/addon-interactions": "^8.0.10", | ||
"@storybook/addon-links": "^8.0.10", | ||
"@storybook/blocks": "^8.0.10", | ||
"@storybook/nextjs": "^8.0.10", | ||
"@storybook/react": "^8.0.10", | ||
"@storybook/test": "^8.0.10", | ||
"@types/mock-fs": "^4.13.4", | ||
"@types/node": "^20", | ||
"@types/react": "^18", | ||
|
@@ -40,6 +49,7 @@ | |
"jest": "^29.7.0", | ||
"mock-fs": "^5.2.0", | ||
"prettier": "^3.1.1", | ||
"storybook": "^8.0.10", | ||
"ts-jest": "^29.1.2", | ||
"typescript": "^5" | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,3 +33,5 @@ yarn-error.log* | |
# typescript | ||
*.tsbuildinfo | ||
next-env.d.ts | ||
|
||
*storybook.log | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. new line 🙏🏼 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { join, dirname } from 'path'; | ||
|
||
/** | ||
* This function is used to resolve the absolute path of a package. | ||
* It is needed in projects that use Yarn PnP or are set up within a monorepo. | ||
*/ | ||
function getAbsolutePath(value) { | ||
return dirname(require.resolve(join(value, 'package.json'))); | ||
} | ||
|
||
/** @type { import('@storybook/nextjs').StorybookConfig } */ | ||
const config = { | ||
stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|mjs|ts|tsx)'], | ||
addons: [ | ||
getAbsolutePath('@storybook/addon-links'), | ||
getAbsolutePath('@storybook/addon-essentials'), | ||
getAbsolutePath('@chromatic-com/storybook'), | ||
getAbsolutePath('@storybook/addon-interactions'), | ||
], | ||
framework: { | ||
name: getAbsolutePath('@storybook/nextjs'), | ||
options: {}, | ||
}, | ||
docs: { | ||
autodocs: 'tag', | ||
}, | ||
}; | ||
export default config; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** @type { import('@storybook/react').Preview } */ | ||
const preview = { | ||
parameters: { | ||
controls: { | ||
matchers: { | ||
color: /(background|color)$/i, | ||
date: /Date$/i, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
export default preview; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,9 @@ | |
"lint:eslint": "eslint .", | ||
"lint:eslint-fix": "eslint . --fix", | ||
"lint": "next lint", | ||
"test": "jest" | ||
"test": "jest", | ||
"storybook": "storybook dev -p 6006", | ||
"build-storybook": "storybook build" | ||
}, | ||
"dependencies": { | ||
"@emotion/react": "^11.11.1", | ||
|
@@ -23,5 +25,16 @@ | |
"simple-git": "^3.20.0", | ||
"yaml": "^2.3.3", | ||
"zod": "^3.22.4" | ||
}, | ||
"devDependencies": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible to move all dev dep to the root package.json? so we keep sub projects free from dev dependencies |
||
"@chromatic-com/storybook": "^1.3.5", | ||
"@storybook/addon-essentials": "^8.0.10", | ||
"@storybook/addon-interactions": "^8.0.10", | ||
"@storybook/addon-links": "^8.0.10", | ||
"@storybook/blocks": "^8.0.10", | ||
"@storybook/nextjs": "^8.0.10", | ||
"@storybook/react": "^8.0.10", | ||
"@storybook/test": "^8.0.10", | ||
"storybook": "^8.0.10" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import { Cache } from '@/Cache'; | ||
import HomeDashboard from '@/components/HomeDashboard'; | ||
import MessageAdapterFactory from '@/utils/adapters/MessageAdapterFactory'; | ||
import { ProjectCardProps } from '@/components/ProjectCard'; | ||
import { RepoGit } from '@/RepoGit'; | ||
import { ServerConfig } from '@/utils/serverConfig'; | ||
|
||
// Force dynamic rendering for this page. By default Next.js attempts to render | ||
// this page statically. That means that it tries to render the page at build | ||
// time instead of at runtime. That doesn't work: this page needs to fetch | ||
// project-specific config files and perform git operations. So this little | ||
// one-liner forces it into dynamic rendering mode. | ||
// | ||
// More info on dynamic vs static rendering at: | ||
// https://nextjs.org/learn/dashboard-app/static-and-dynamic-rendering | ||
// | ||
// More info on `export const dynamic` at: | ||
// https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config | ||
export const dynamic = 'force-dynamic'; | ||
|
||
export default async function Home() { | ||
const serverConfig = await ServerConfig.read(); | ||
const projects = await Promise.all( | ||
serverConfig.projects.map<Promise<ProjectCardProps>>(async (project) => { | ||
await RepoGit.cloneIfNotExist(project); | ||
const repoGit = await RepoGit.getRepoGit(project); | ||
const lyraConfig = await repoGit.getLyraConfig(); | ||
const projectConfig = lyraConfig.getProjectConfigByPath( | ||
project.projectPath, | ||
); | ||
const msgAdapter = MessageAdapterFactory.createAdapter(projectConfig); | ||
const messages = await msgAdapter.getMessages(); | ||
const store = await Cache.getProjectStore(projectConfig); | ||
const languages = await Promise.all( | ||
projectConfig.languages.map(async (lang) => { | ||
const translations = await store.getTranslations(lang); | ||
return { | ||
href: `/projects/${project.name}/${lang}`, | ||
language: lang, | ||
progress: translations | ||
? (Object.keys(translations).length / messages.length) * 100 | ||
: 0, | ||
}; | ||
}), | ||
); | ||
|
||
return { | ||
href: `/projects/${project.name}`, | ||
languages, | ||
messageCount: messages.length, | ||
name: project.name, | ||
}; | ||
}), | ||
); | ||
|
||
return <HomeDashboard projects={projects} />; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import { FC } from 'react'; | ||
import { Box, List, Typography } from '@mui/joy'; | ||
import ProjectCard, { ProjectCardProps } from './ProjectCard'; | ||
|
||
type HomeDashboardProps = { | ||
projects: ProjectCardProps[]; | ||
}; | ||
|
||
/** | ||
*/ | ||
const HomeDashboard: FC<HomeDashboardProps> = ({ projects }) => { | ||
return ( | ||
<Box | ||
alignItems="center" | ||
display="flex" | ||
flexDirection="column" | ||
justifyContent="center" | ||
minHeight="97vh" | ||
> | ||
<Typography alignSelf="flex-start" color="primary" component="h1"> | ||
Your Lyra Projects | ||
</Typography> | ||
<List | ||
sx={{ | ||
'@media (min-width: 600px)': { | ||
alignContent: 'center', | ||
columnGap: 2, | ||
display: 'grid', | ||
gridTemplateColumns: 'repeat(auto-fit, minmax(200px, 300px));', | ||
justifyContent: 'center', | ||
}, | ||
alignItems: 'center', | ||
display: 'flex', | ||
flex: 1, | ||
flexDirection: 'column', | ||
rowGap: 2, | ||
width: '100%', | ||
}} | ||
> | ||
{projects.map((project, i) => ( | ||
<ProjectCard key={i} {...project} /> | ||
))} | ||
</List> | ||
</Box> | ||
); | ||
}; | ||
|
||
export default HomeDashboard; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 am okay with yarn 4, but afaik the other Zetkin projects user yarn classic. and want to keep it this way
@richardolsson any comments?
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.
Thanks for raising this @amerharb! Here's some background.
When we started the Gen3 web frontend (zetkin/app.zetkin.org) I did some research and found that lots of people (including some major organizations like Facebook and others) had decided to stick with yarn classic because they found some design decisions made in yarn2 to be offputting. I can't even remember fully why now, but I believe it had something to do with dependency management.
So we decided to also stick with yarn classic. But it's been years, and I'm open to reevaluating using yarn4 in both projects. Maybe Lyra can be a good ground for experiments. Clearly it's helping to fix a problem here at least.
But we're going to be needing help migrating (and understanding the repercussions of migrating) the Gen3 webapp also at some point then. Maybe @henrycatalinismith who's familiar with both can commit to that? That would make me feel a bit safer in taking this step.
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.
For more context about this change, check out the following:
I tried pretty hard to avoid the yarn v4 upgrade. Of the workarounds listed in those issues, it was the only one that worked. It's still not our only option though. Here are some alternatives:
webapp
directory.This yarn v1 bug is only triggered by moving those dependencies from the webapp's package.json to the root package.json. It works fine if we install storybook directly in the webapp.
If a yarn migration is undesirable and/or labour-intensive, and we're equally not keen on allowing devDependencies inside webapp/package.json, we can satisfy both of those constraints by simply not using Storybook at all. I'm completely fine with this option, for what it's worth.
Let me know what you think! Personally my fav option tends to be the path of least resistance. So I'd be more than happy to ditch Storybook from the scope of this PR and walk back the yarn upgrade.