Skip to content
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

Conversation

henrycatalinismith
Copy link
Collaborator

@henrycatalinismith henrycatalinismith commented May 13, 2024

Fixes #74. Addresses some of #71 too.

Project Card

Default state Project hover Language hover
Screenshot 2024-05-13 at 22 21 58 Screenshot 2024-05-13 at 22 22 03 Screenshot 2024-05-13 at 22 22 09

Home Screen

With just one project With several projects
Screenshot 2024-05-13 at 22 19 49 Screenshot 2024-05-13 at 22 19 35

@henrycatalinismith
Copy link
Collaborator Author

Much nicer now.

@@ -23,5 +25,16 @@
"simple-git": "^3.20.0",
"yaml": "^2.3.3",
"zod": "^3.22.4"
},
"devDependencies": {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

@henrycatalinismith henrycatalinismith force-pushed the issue-74-project-card branch 3 times, most recently from 1df1e16 to 1afa125 Compare May 25, 2024 12:40
@@ -3,6 +3,7 @@
"version": "0.0.1",
"description": "Root package of Lyra",
"private": true,
"packageManager": "[email protected]",
Copy link
Collaborator

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?

Copy link
Member

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.

Copy link
Collaborator Author

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:

  • Move the Storybook devDependencies back into the 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.
  • Remove Storybook.
    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.

@@ -33,3 +33,5 @@ yarn-error.log*
# typescript
*.tsbuildinfo
next-env.d.ts

*storybook.log
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new line 🙏🏼

@henrycatalinismith
Copy link
Collaborator Author

Reflected all day on my summary of the options in #86 (comment). Decided to pick my favourite and make it happen. So I'm closing this in favour of #91!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project Card
3 participants