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

Fix: Add 'loading' message to settings page #2806

Merged

Conversation

boc-the-git
Copy link
Collaborator

@boc-the-git boc-the-git commented Dec 9, 2023

What type of PR is this?

  • bug
  • cleanup

What this PR does / why we need it:

Currently when loading the settings, there is just nothing while waiting for the app info to load. The populates a Loading... message.

Which issue(s) this PR fixes:

None.

Notes for Reviewer

The git diff looks to make it fairly hard to compare the actual scope of the change, which was only a few lines. It could be easier to look at the old and new version side by side, or in a better comparison tool.

Testing

Mealie Settings Loading

@boc-the-git boc-the-git changed the title Add 'loading' message to settings page Fix: Add 'loading' message to settings page Dec 9, 2023
@michael-genson
Copy link
Collaborator

A couple things:

  • Can we add some y padding to bottom to center it in the card? It looks a little crunched.
  • Alternatively you might want to use the AppLoader component which gives a nice animation. For example, on the global timeline page:
    2023-12-09_09h05_55

<AppLoader :loading="loading" :waiting-text="$tc('general.loading-events')" />

@boc-the-git
Copy link
Collaborator Author

Good suggestion, I've implemented the AppLoader.

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.

LGTM!

@michael-genson michael-genson merged commit 7aac82b into mealie-recipes:mealie-next Dec 10, 2023
6 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.

2 participants