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/recipe timeline event UI #1831

Merged

Conversation

michael-genson
Copy link
Collaborator

What type of PR is this?

(REQUIRED)

  • bug
  • feature

What this PR does / why we need it:

(REQUIRED)

This implements a new UI for recipe timeline events, and fixes a few bugs and visual glitches with the new "I Made This" functionality (#1801). With these changes we more-or-less complete #1451, save for implementing image handling (this PR is big enough already, I'll add that later).

To open the new UI, there's a new badge:
image

Here's what it looks like on desktop:
image

And on mobile we use the dense view:
image

If you are the "owner" of the event (your userId matches the event's userId") and the eventType is not system, then there's a context menu that lets you edit or delete the event:
image
image
image

If no events are on the recipe, you get a generic message instead. Note that this shouldn't happen with new recipes, as they always create an event since that API was implemented. To trigger this I had to delete the default event through the API.
image

The already-implemented "I Made This" UI rendered horribly on mobile, so that has been updated.
Desktop:
image

Mobile:
image

Lastly, I made all of this text localizable.

Which issue(s) this PR fixes:

(REQUIRED)

Completes FR #1451

Special notes for your reviewer:

Ooh boy did I learn a lot about Vue and JS working on this. Happy to fix/refactor anything that doesn't make sense or can look better. I tried to mirror the existing components as best as possible.

Did I implement the localizations correctly? Will the JSON be automatically updated next time the Crowdin code runs? I mirrored a different PR I found that added new localizations, and I think I did it right.

In a few places I had to do some cheeky stuff to convert between local datetimes and UTC datetimes. What I did definitely works, but my Python brain had trouble wrapping my head around how JS handles datetimes. It's worth reviewing and seeing if I cooked too much pasta with the datetime stuff, specifically in RecipeLastMade.vue and RecipeDialogTimeline.vue.

Testing

(fill-in or delete this section)

I tested the following functionalities:

  • Using "I Made This" to generate new events
  • Using the API to generate other events
  • Editing and deleting existing events via the context menu (and confirming the changes are reflected without a refresh)

I tested the following elements:

  • Desktop rendering (various screen sizes)
  • Mobile rendering (various screen sizes)
  • Having a lot of events
  • Having few events
  • Having no events

Release Notes

(REQUIRED)

added new recipe timeline interface

Copy link
Collaborator

@hay-kot hay-kot left a comment

Choose a reason for hiding this comment

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

A few small things in the frontend but looks good overall. Given the previous merged, does anything here need to change?

@michael-genson
Copy link
Collaborator Author

A few small things in the frontend but looks good overall. Given the previous merged, does anything here need to change?

Thanks! I'll take a look at your comments - looking briefly they make sense.
Regarding other merges I don't think anything needs to change but I'll merge and run through everything and confirm

@michael-genson
Copy link
Collaborator Author

I ended out changing the mobile view a bit to more closely match the desktop view, and I think it looks better:
image

I replaced most of the different attributes using an attrs object like you suggested, but I had to use some ternaries for other attributes (like col)

@michael-genson
Copy link
Collaborator Author

@hay-kot this should be ready :)

@hay-kot hay-kot merged commit 4e8e2d7 into mealie-recipes:mealie-next Dec 11, 2022
@michael-genson michael-genson deleted the feat/recipe-timeline-event-ui branch December 11, 2022 21:25
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.

2 participants