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: Shopping list UI overhaul - add wakelock #4236

Merged
merged 10 commits into from
Sep 28, 2024

Conversation

Wetzel402
Copy link
Contributor

What type of PR is this?

  • feature

What this PR does / why we need it:

  • Add keep screen awake

Which issue(s) this PR fixes:

#4077

Testing

Tested in dev container

Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Thanks for spliting all this up. Makes it super easy to review 😊👍
Since this is all the same code as on the recipe page, could you create a component that includes the switch as well as the functionality and just throw this component on both pages?
Decreases duplicate code and makes the code of those pages a bit less convoluted.

@Kuchenpirat Kuchenpirat self-assigned this Sep 19, 2024
@Wetzel402
Copy link
Contributor Author

Thanks for spliting all this up. Makes it super easy to review 😊👍 Since this is all the same code as on the recipe page, could you create a component that includes the switch as well as the functionality and just throw this component on both pages? Decreases duplicate code and makes the code of those pages a bit less convoluted

That makes sense. It might take me a bit to figure that out since I'm new to vue.

@Kuchenpirat
Copy link
Collaborator

Kuchenpirat commented Sep 19, 2024

No worries. If you want to tackle this for fun all good and take your time, but if this is just work for you i am happy to throw this together in 5 minutes and add this to this PR.

	new file:   frontend/components/global/Wakelock.vue
	modified:   frontend/pages/shopping-lists/_id.vue
@Wetzel402
Copy link
Contributor Author

No worries. If you want to tackle this for fun all good and take your time, but if this is just work for you i am happy to throw this together in 5 minutes and add this to this PR.

Maybe I'm a masochist but I enjoy putting myself through learning new ways to code...haha

I've pushed the changes for your review.

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.

Check the files changed tab on GH; a few linter warnings got raised. I also added a few comments

frontend/components/global/Wakelock.vue Outdated Show resolved Hide resolved
Wetzel402 and others added 2 commits September 20, 2024 14:34
	renamed:    frontend/components/global/Wakelock.vue -> frontend/components/global/WakelockSwitch.vue
	modified:   frontend/pages/shopping-lists/_id.vue
Kuchenpirat
Kuchenpirat previously approved these changes Sep 22, 2024
Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀
Thanks for your work on this!

Ah right, i missed the code warnings.

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.

There's still a few warnings to take care of; check out the "Files changed" tab on GH and scroll down and you'll see them

@Wetzel402
Copy link
Contributor Author

There's still a few warnings to take care of; check out the "Files changed" tab on GH and scroll down and you'll see them

Sorry for the delay. PR should be ready now.

@Wetzel402 Wetzel402 closed this Sep 26, 2024
@Wetzel402 Wetzel402 deleted the wakelock branch September 26, 2024 21:00
@Kuchenpirat
Copy link
Collaborator

@Wetzel402 any reason you closed this?

@Wetzel402 Wetzel402 restored the wakelock branch September 27, 2024 14:54
@Wetzel402 Wetzel402 reopened this Sep 27, 2024
@Wetzel402
Copy link
Contributor Author

@Wetzel402 any reason you closed this?

Sorry, I thought it had been merged already and was doing some pruning on my repository.

Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Looks all good 👍

@michael-genson michael-genson enabled auto-merge (squash) September 28, 2024 15:07
@michael-genson michael-genson merged commit 28b0190 into mealie-recipes:mealie-next Sep 28, 2024
13 checks passed
@Wetzel402 Wetzel402 deleted the wakelock branch September 28, 2024 15:16
@boc-the-git
Copy link
Collaborator

Thanks for this @Wetzel402; it was useful to me today! :)

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.

4 participants