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

Open
wants to merge 4 commits into
base: mealie-next
Choose a base branch
from

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
	renamed:    frontend/components/global/Wakelock.vue -> frontend/components/global/WakelockSwitch.vue
	modified:   frontend/pages/shopping-lists/_id.vue
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.

3 participants