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: Add 'No Shopping Lists Found' message #4661

Merged

Conversation

niteflyunicorns
Copy link
Contributor

What type of PR is this?

feature

What this PR does / why we need it:

Currently, if there are no shopping lists, the Shopping Lists page just shows a blank screen. This adds a message where the shopping lists would show if they existed that says "No Shopping Lists Found".

  • Adds "No Shopping Lists Found" message to the "Shopping List" page if no lists exist
  • Adds i18-n translations for the message

Before:
before

After:
after

Testing

Since this was a UI improvement, I just observed the changes visually.

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! Please remove the changes to lang/messages other than the en-US.json file, the rest is handled automatically through crowdin.

@niteflyunicorns
Copy link
Contributor Author

Ahh sorry, didn't realize it was handled automatically! Fixed it.

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

Just a small change on the text. Can we also add this to frontend/components/Domain/Recipe/RecipeDialogAddToShoppingList.vue?

@@ -872,7 +872,8 @@
"you-are-offline-description": "Not all features are available while offline. You can still add, modify, and remove items, but you will not be able to sync your changes to the server until you are back online.",
"are-you-sure-you-want-to-check-all-items": "Are you sure you want to check all items?",
"are-you-sure-you-want-to-uncheck-all-items": "Are you sure you want to uncheck all items?",
"are-you-sure-you-want-to-delete-checked-items": "Are you sure you want to delete all checked items?"
"are-you-sure-you-want-to-delete-checked-items": "Are you sure you want to delete all checked items?",
"no-shopping-list-found": "No Shopping Lists Found."
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 this looks better without the period, since it's standalone text. Also want to make sure the key matches the text

Suggested change
"no-shopping-list-found": "No Shopping Lists Found."
"no-shopping-lists-found": "No Shopping Lists Found"

@@ -21,6 +21,12 @@
<BaseButton create @click="createDialog = true" />
</v-container>

<v-container v-if="shoppingListChoices.length === 0">
<BasePageTitle>
<template #title>{{ $t('shopping-list.no-shopping-list-found') }}</template>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<template #title>{{ $t('shopping-list.no-shopping-list-found') }}</template>
<template #title>{{ $t('shopping-list.no-shopping-lists-found') }}</template>

@@ -21,6 +21,12 @@
<BaseButton create @click="createDialog = true" />
</v-container>

<v-container v-if="shoppingListChoices.length === 0">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a nitpick, no need to change, but for the sake of learning FYI this also works:

<v-container v-if="!shoppingListChoices.length">

It's preference, but this way is more typical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's great feedback, I will change it. I'm new to Vue so every bit of "for the sake of learning" is helpful, thank you.

@niteflyunicorns
Copy link
Contributor Author

I think I got all the feedback, but let me know if I missed anything!

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@michael-genson michael-genson merged commit d9a1db5 into mealie-recipes:mealie-next Dec 5, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants