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

[feat] view plants: created PlantCard component #4

Merged

Conversation

SashankBalusu
Copy link
Contributor

@SashankBalusu SashankBalusu commented Oct 6, 2024

What's new in this PR 🧑‍🌾

Description

The main changes were adding the plant card component and supabase function call (the function that is called in this file is also viewable on supabase)

  • the styling for the plant card components is still rough as the design is changing + moving to styled components will be a later sprint task, so the important stuff is mainly the html

Screenshots

Screenshot 2024-10-08 at 5 44 23 PM

Rough styling --still need to add icons, etc

How to review

  • for supabase functions, the order is probably to look at the supabase function in the actually database and then the syntax here.
  • for plant card component again the styling will be changed, but would appreciate feedback on the html structure

Next steps

  • styling as mentioned a couple times before

Relevant links

Online sources

https://stackoverflow.com/questions/55688418/what-is-a-non-setof-function-in-postgresql

Related PRs

CC: @ccatherinetan

Copy link
Collaborator

@ccatherinetan ccatherinetan left a comment

Choose a reason for hiding this comment

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

Looking great so far! Would it be possible to create an instance of this component in the app/view-plants directory to test it out?
Also make sure to run npm run lint and npm run prettier:fix before you push to pass the styling checks!

@@ -0,0 +1,52 @@
.Card {
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks so much for starting the initial styling! can we migrate these styles to Styled Components?
see kevin's pr for reference on using Styled Components!
Styled components doc: https://styled-components.com/
Here's a styles.tsx file reference from IJP: https://github.com/calblueprint/immigration-justice-project/blob/main/src/components/ListingDetails/styles.ts

SashankBalusu pushed a commit that referenced this pull request Oct 9, 2024
* update sort order, use pnpm, update styles

* update README

* update workflow
@SashankBalusu SashankBalusu changed the title Added database function + basic plant card component [feat] view plants: created PlantCard component Oct 9, 2024
0 24px 38px 3px rgba(0, 0, 0, 0.14),
0 9px 46px 8px rgba(0, 0, 0, 0.12),
0 11px 15px -7px rgba(0, 0, 0, 0.2);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

image
The text goes outside of the div depending on the browser zoom, maybe consider setting an 'overflow' attribute? or setting a fixed font size for the text since it currently changes depending on the zoom

border-top-left-radius: 12px;
border-top-right-radius: 12px;
}
.cardContent {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would probably change anyways when you switch to styled components but style class names should use Pascal Case (e.g. CardContent) instead of Camel Case (but this is a small thing lol)

@ccatherinetan
Copy link
Collaborator

great work! i think the final changes are

  1. Removing the src/directory.
  2. Fixing the way we're passing props for PlantCard (see my comment; we prob don't want to use the spread operator)
  3. Try to actually test your get_plant_by_id query to display a PlantCard in view-plants
    Kyle left some great comments about the styling; if you have time that would be great to look at, but we're prioritizing backend functionality first. So if you could address these 3 things, we can merge it in!

@ccatherinetan ccatherinetan force-pushed the sashankbalusu/tg-16-create-the-plantcard-component branch 2 times, most recently from 743b29b to 1077750 Compare October 21, 2024 22:37
@ccatherinetan ccatherinetan merged commit 9fc4041 into main Oct 21, 2024
4 checks passed
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.

3 participants