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

[HUM-127]: chore: refactor page card components #2987

Conversation

mpblocky
Copy link
Contributor

@mpblocky mpblocky commented Jan 10, 2025

Issue tracking

HUM-127

Context behind the change

How has this been tested?

  • withLayoutBackground - manually checked on mocked page, how do each of page-card*.tsx look
  • manual checks for other changes

Release plan

normal deploy

Potential risks; What to monitor; Rollback plan

N/A

Copy link

vercel bot commented Jan 10, 2025

@mpblocky is attempting to deploy a commit to the HUMAN Protocol Team on Vercel.

A member of the Team first needs to authorize it.

@mpblocky mpblocky changed the title Hum 127 refactor components refactor page card components [HUM-127]: chore: refactor page card components Jan 13, 2025
@mpblocky mpblocky marked this pull request as ready for review January 13, 2025 09:10
@mpblocky
Copy link
Contributor Author

@dnechay ready for review

@dnechay dnechay requested review from dnechay and removed request for adrian-oleskiewicz January 13, 2025 10:03
Copy link
Contributor

@dnechay dnechay left a comment

Choose a reason for hiding this comment

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

Let's have page-card/index.ts that re-exports necessary components, so they can be imported easily in on import statement, i.e.

import {
  PageCardError,
  PageCardLoader
} from '@/shared/components/ui/page-card';

instead of

import { PageCardError } from '@/shared/components/ui/page-card/page-card-error';
import { PageCardLoader } from '@/shared/components/ui/page-card/page-card-loader';

@mpblocky mpblocky force-pushed the HUM-127-refactor-components-refactor-page-card-components branch from a23d85e to 60fc02f Compare January 22, 2025 08:47
@mpblocky
Copy link
Contributor Author

@dnechay this one is ready for re-review :)

@dnechay dnechay self-requested a review January 22, 2025 14:38
Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
human-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 27, 2025 0:26am

Copy link
Contributor

@dnechay dnechay left a comment

Choose a reason for hiding this comment

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

LGTM overall
@mpblocky few small code-style suggestions

@mpblocky
Copy link
Contributor Author

@dnechay applied suggestions, I think it looks good now :)

@dnechay dnechay self-requested a review January 24, 2025 14:36
@mpblocky
Copy link
Contributor Author

All done @dnechay

Copy link
Contributor

@dnechay dnechay left a comment

Choose a reason for hiding this comment

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

Nice!

@dnechay dnechay merged commit 4c31445 into humanprotocol:develop Jan 27, 2025
5 of 9 checks passed
@dnechay dnechay mentioned this pull request Jan 28, 2025
35 tasks
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.

4 participants