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] profile context integration with view plants and add details #63

Merged

Conversation

SashankBalusu
Copy link
Contributor

@SashankBalusu SashankBalusu commented Dec 7, 2024

What's new in this PR 🧑‍🌾

Description

  • refactored View Plants into MyPlantsDisplay, AllPlantsDisplay, loading screens, and not logged in / not onboarded screens
  • styled add-details roughly, using CustomizedInput (from onboarding)
    • added additional prop to make the entire input clickable

Screenshots

enabled next (when planting type is filled out)
Screen Shot 2024-12-13 at 1 58 58 AM

disabled next
Screen Shot 2024-12-13 at 1 58 06 AM

How to review

  • maily look at add details and view plants and play around with it

Next steps

  • View plants
    • Possibly add a state for fetchUserPlantsLoading? (We can infer fetchPlantsLoading based on length of plants) → then, we can display a loading screen if the plants haven’t been fetched yet.
  • Add Plants:
    • elegantly handle not logged in case (e.g. when someone clicks "Back") instead of doing userId!
    • Fix the default Date (make sure it aligns with US Eastern Time)
    • Add images for plants
    • Displaying + Styling the Review Page for Add Plants
  • CustomizedInput: refactor away the optional prop / resolve merge conflicts with onboarding
    • rename label prop to placeholder
    • add an actual label prop that displays the label of the input

Relevant links

Online sources

Related PRs

CC: @ccatherinetan

@SashankBalusu SashankBalusu linked an issue Dec 7, 2024 that may be closed by this pull request
@ccatherinetan ccatherinetan changed the title profile context integration with view plants and add detaisl [feat] profile context integration with view plants and add details Dec 7, 2024
@ccatherinetan ccatherinetan force-pushed the 58-use-profilecontext-in-view-plants-and-add-details branch from ee84673 to 407fa28 Compare December 7, 2024 22:14
@SashankBalusu SashankBalusu marked this pull request as ready for review December 13, 2024 05:11
@ccatherinetan ccatherinetan force-pushed the 58-use-profilecontext-in-view-plants-and-add-details branch from 869222b to 7d8ef47 Compare December 13, 2024 09:11
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! will leave some comments about next steps

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we remove this file if we're not using it anywhere?

);
const router = useRouter();

const getDefaultDate = () => new Date().toISOString().substring(0, 10);
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 date is a little funky. it's 1:48am 12/13, but the plant i just added says 12/12.

Perhaps we should look into other date formatting options e.g. toLocaleDateString() and set it to US Eastern Time, since that's where users will be.

const router = useRouter();

if (profileReady && !profileData) {
router.push('/view-plants');
Copy link
Collaborator

Choose a reason for hiding this comment

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

note: we should replace all routes with CONFIG
e.g.

import CONFIG from '@/lib/configs';
<Link href={CONFIG.login}>Login</Link>

@ccatherinetan ccatherinetan merged commit ee776c9 into main Dec 13, 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.

Use ProfileContext in View Plants and Add Details
2 participants