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] style view-plants #56

Merged
merged 15 commits into from
Dec 2, 2024
Merged

[feat] style view-plants #56

merged 15 commits into from
Dec 2, 2024

Conversation

SashankBalusu
Copy link
Contributor

@SashankBalusu SashankBalusu commented Nov 27, 2024

What's new in this PR 🧑‍🌾

Description

  • styled the /view-plants page
    • added buttons: Select/Cancel + Add To Plants in Add Mode (Add To Plants only routes to /add-details, but doesn't send over the selected plants)
  • made PlantCard a memoized component: this prevents unnecessary re-renders when filteredPlantList changes, but the plants themselves don't change.
  • created a util function to map months to seasons
  • added styles/containers.ts for generic Box and Flex containers

Screenshots

My Garden
Screen Shot 2024-12-01 at 6 47 56 PM

All Plants
Screen Shot 2024-12-01 at 6 48 18 PM
Screen Shot 2024-12-01 at 6 48 46 PM
Screen Shot 2024-12-01 at 6 49 01 PM

How to review

  • see /view-plants, components/PlantCard/
  • check that the selecting/canceling in add mode works; try toggling between My and All Plants

Next steps

styling

  • add emptyCircle and checkedCircle as icons (rather than using RoundCheck)
  • fix the view-plants grid to ensure that all phone screens can fit 2 cards per row (may need to decrease the card size to 132px)
    • play with the aspect ratio attribute (for PlantCard)
  • confirm whether we should center the plant cards (i.e. finalize mobile and desktop screens for /view-plants)
  • standardize text sizes/styles + consolidate styles

functionality

  • connect view-plants with add-details using ProfileContext
  • make viewingOption state to a boolean state?

Relevant links

Online sources

Related PRs

CC: @ccatherinetan

@ccatherinetan ccatherinetan linked an issue Nov 27, 2024 that may be closed by this pull request
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 good so far! a note about error handling in mapMonthToSeason - will check back in after additional styling!

utils/helpers.ts Outdated Show resolved Hide resolved
utils/helpers.ts Outdated
@@ -227,3 +227,26 @@ export function formatTimestamp(timestamp: string): string {
// Return in MM/DD/YYYY format
return `${month}/${day}/${year}`;
}
export function mapMonthToSeason(month: string): string {
month = processPlantMonth(month).toUpperCase();
const monthToSeason: { [key: string]: string } = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can move this map outside the function in case we need to use it elsewhere?

app/view-plants/page.tsx Outdated Show resolved Hide resolved
components/DifficultyLevelBar/style.ts Outdated Show resolved Hide resolved
align-items: center;
width: 136px;
`;
export const PlantImage = styled.img`
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we should be using styled(NextImage) instead of styled.img

Comment on lines +17 to +19
0 24px 38px 3px rgb(148, 181, 6, 0.14),
0 9px 46px 8px rgb(148, 181, 6, 0.12),
0 11px 15px -7px rgb(148, 181, 6, 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.

checking w/ kyrene if we want this!

background: none;
border: none;
color: ${COLORS.shrub};
font-family: inherit;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be by default!

app/view-plants/page.tsx Outdated Show resolved Hide resolved
Comment on lines +24 to +27
font-size: 16px;
font-style: normal;
font-weight: 400;
line-height: normal;
Copy link
Collaborator

Choose a reason for hiding this comment

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

note, this should be a styled(P1), which would handle the font styling for you. then, you can use HeaderButton with as="button", i.e.
<HeaderButton as="button"> </HeaderButton>

Pull the latest commits on main to see the P1 style in styles/text.ts!

components/PlantCard/index.tsx Outdated Show resolved Hide resolved
@ccatherinetan ccatherinetan marked this pull request as ready for review December 2, 2024 01:17
@ccatherinetan ccatherinetan changed the title plant card fixes and grid view [feat] style view-plants Dec 2, 2024
@ccatherinetan ccatherinetan merged commit a772f80 into main Dec 2, 2024
2 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.

Style View Plants
2 participants