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

✨ Enhancement: Create a card component to be able to standarize the usage of list of cards #433

Closed
benjagm opened this issue Mar 3, 2024 · 16 comments · Fixed by #460
Closed
Assignees
Labels
✨ Enhancement Indicates that the issue suggests an improvement or new feature. Status: In Progress This issue is being worked on, and has someone assigned.

Comments

@benjagm
Copy link
Collaborator

benjagm commented Mar 3, 2024

Is your feature request related to a problem? Please describe

No

Describe the solution you'd like

We are working in different issue that require usage of cards like #426 and #421 and it will be beneficial to have a standard card component to be able to use across.

Examples for reference:
Use Cases
Screenshot 2024-03-03 at 16 21 37
Docs Landing
Screenshot 2024-03-03 at 15 16 34

I think the best approach is follow the design used in these card of the landing page:

Screenshot 2024-03-03 at 23 20 16

The card should support:

  • Mandatory: Title
  • Mandatory: Body
  • Optional: Icon
  • Optional: Link

Describe alternatives you've considered

No response

Additional context

No response

Are you working on this?

No

@benjagm benjagm added ✨ Enhancement Indicates that the issue suggests an improvement or new feature. Status: Triage This is the initial status for an issue that requires triage. Status: Available No one has claimed responsibility for resolving this issue. and removed Status: Triage This is the initial status for an issue that requires triage. labels Mar 3, 2024
@benjagm
Copy link
Collaborator Author

benjagm commented Mar 3, 2024

@TamannaVerma99 and @VivekJaiswal18 please use this card component for your development.

@VivekJaiswal18
Copy link
Contributor

Okay thats good!

@VivekJaiswal18
Copy link
Contributor

@benjagm where should we add the navigation button for routing

@TamannaVerma99
Copy link
Contributor

okay great!!

@divyaxdv
Copy link
Contributor

divyaxdv commented Mar 4, 2024

@benjagm can you please assign this to me? I am very much intrested in contributing to it.

@Michael-Obele
Copy link
Contributor

Is anyone assigned to this? I'd love to do it or collaborate with them.

@benjagm
Copy link
Collaborator Author

benjagm commented Mar 4, 2024

@Michael-Obele please go ahead with this one. Thanks.

@benjagm benjagm added Status: In Progress This issue is being worked on, and has someone assigned. and removed Status: Available No one has claimed responsibility for resolving this issue. labels Mar 4, 2024
@TamannaVerma99
Copy link
Contributor

Hey! @benjagm
As I have to use this component to proceed with the welcome page issue so should I wait for this issue to get completed first?

@divyaxdv
Copy link
Contributor

divyaxdv commented Mar 4, 2024

@Michael-Obele let me know if I could assist you in any way. I would be pleased to help .

@Michael-Obele
Copy link
Contributor

@Michael-Obele let me know if I could assist you in any way. I would be pleased to help .

I will, thanks.

@Michael-Obele
Copy link
Contributor

Hey guys, I apologize for taking so long to send this PR. I would appreciate your feedback on the design, particularly how it looks with an icon.

@ayushtiwari110
Copy link
Contributor

Hey there @benjagm ,
I just came across this task. The progress done so far is great. I had a suggestion that we can use composition logic for making the Card component. We can refactor the current code for this logic. It'll benefit us in following way:

  • Break Down Functionality: Individual card elements (image, title, description, buttons, etc.) could become smaller components.
  • Compose the Card: The main card component could then compose these smaller pieces together, using props for layout and any essential overarching data.
  • Here's an article for reference: Why and how to use component composition in React
    I guess if the card component would be having a consistent UI across different usecases, we can still go with the current design. But if the design would be varying, I believe building it in compositional way would be helpful in increasing reusability of the card component.
    I would like to hear your opinion on this.

@Michael-Obele
Copy link
Contributor

Hey there @benjagm , I just came across this task. The progress done so far is great. I had a suggestion that we can use composition logic for making the Card component. We can refactor the current code for this logic. It'll benefit us in following way:

  • Break Down Functionality: Individual card elements (image, title, description, buttons, etc.) could become smaller components.
  • Compose the Card: The main card component could then compose these smaller pieces together, using props for layout and any essential overarching data.
  • Here's an article for reference: Why and how to use component composition in React
    I guess if the card component would be having a consistent UI across different usecases, we can still go with the current design. But if the design would be varying, I believe building it in compositional way would be helpful in increasing reusability of the card component.
    I would like to hear your opinion on this.

Hey, thanks for taking a look at the PR! I really appreciate the suggestion about using component composition for the card component. It's a great idea, and I actually started down that path by creating a CardBody component.

Here's why I held off on going too much further with breaking things down:

  • Logic is pretty simple: Right now, the only real logic in the card is for the link; if there's a link, we wrap everything in it. Everything else is pretty straightforward: show it if there's data, hide it if not (except for title and body, which are required).
  • Keeping things uniform: Breaking it down too much might give us a lot of flexibility, but that could also make it harder to keep the cards looking consistent across the app. Uniformity is a big goal here.
  • Keeping it simple: While there are benefits to breaking things down, it can also add complexity. More components might mean more verbose code with lots of props to pass around, and potentially more documentation needed to explain how to use it all.

So, like you said, it is a great idea! I'm definitely open to revisiting this if we start needing more flexibility with card layouts in the future. But for now, I think this approach keeps things clean and easy to manage.

@ayushtiwari110
Copy link
Contributor

ayushtiwari110 commented Mar 8, 2024

@Michael-Obele Yeah I do agree with that. Since our current goal is to keep the cards consistent across the app, we can go ahead with current approach. In future we may revisit to this when we feel there are more requirments with the Card component.

@Michael-Obele
Copy link
Contributor

@Michael-Obele Yeah I do agree with that. Since our current goal is to keep the cards consistent across the app, we can go ahead with current approach. In future we may revisit to this when we feel there are more requirments with the Card component.

Sounds good, thanks for understanding!

@benjagm benjagm added this to the Docs Release 3 milestone Mar 11, 2024
benjagm pushed a commit that referenced this issue Mar 11, 2024
* Standardize List Display with Card Component (#433)

* Addressed comments, added images, and implemented new styles

* Increased padding on the x-axis
@benjagm
Copy link
Collaborator Author

benjagm commented Mar 11, 2024

Closed as completed.

@benjagm benjagm closed this as completed Mar 11, 2024
benjagm pushed a commit that referenced this issue Mar 20, 2024
* Standardize List Display with Card Component (#433)

* Addressed comments, added images, and implemented new styles

* Increased padding on the x-axis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Enhancement Indicates that the issue suggests an improvement or new feature. Status: In Progress This issue is being worked on, and has someone assigned.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants