Skip to content

feat: add Clear History option in Settings #193

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

Jor-Tech
Copy link

Adds a new “Clear History” button to the Settings screen, letting users instantly erase their watch/viewing history. When tapped, the button prompts for confirmation and then clears all stored history entries. This enhancement gives users direct control over their privacy and data with a single, easy-to-find action in the app’s settings.

@Fredolx
Copy link
Owner

Fredolx commented Apr 22, 2025

I'm really impressed with your work. This is the highest quality PR I've received. Just so you know, since the app is commercially availaible on the Microsoft Store and soon the Apple Store, I will have to make you sign some form of CLA.

Thank you again!

@Jor-Tech
Copy link
Author

Thanks. It's just a small contribution but I think some can appreciate the feature.

Please provide me the CLA to sign it.

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2025

CLA assistant check
All committers have signed the CLA.

@Jor-Tech
Copy link
Author

Signed

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

Hey @Jor-Tech, I left a comment, if you could amend your PR. Thank you!

@Jor-Tech
Copy link
Author

@Fredolx where can I find your comment I don't see it?

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

Here's a link to it @Jor-Tech
#193 (comment)

@Jor-Tech
Copy link
Author

Here's a link to it @Jor-Tech #193 (comment)

Sorry, I really want to follow up on your comment, but the link you point to doesn't work for me, and when I got to the "Files changed" tab I also don't find any comments on the code.

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

I will copy paste the comment

In src/app/settings/settings.component.html:

You forgot to commit the css class .history, the svg does not show. Also the tooltip is unnecessary. You can remove it.

image

@Jor-Tech

@Jor-Tech
Copy link
Author

I will copy paste the comment

In src/app/settings/settings.component.html:

You forgot to commit the css class .history, the svg does not show. Also the tooltip is unnecessary. You can remove it.

image

@Jor-Tech

Thanks, updated the PR with the requested changes in your comment.

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

I reviewed a bit the look and there's a few final touch-ups to make. To match the rest of the UI, move the new button before "Delete everything" so it makes sense logically. Then also change it from btn-warning to btn-secondary so that the color of the text is also white.

After that we should be ready to merge.

@Jor-Tech
Copy link
Author

I reviewed a bit the look and there's a few final touch-ups to make. To match the rest of the UI, move the new button before "Delete everything" so it makes sense logically. Then also change it from btn-warning to btn-secondary so that the color of the text is also white.

After that we should be ready to merge.

image

Do you mind white text on yellow button, prefer to have the button style exactly the same as the "Refresh all" button (white text on grey button), or make it black text on yellow button like before? Personally I would go with the latter, but up to you of course.

Let me know.

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

white text on yellow button looks quite ugly, so I prefer the button to be secondary

@Jor-Tech
Copy link
Author

white text on yellow button looks quite ugly, so I prefer the button to be secondary

Agree, that's why I thought black on yellow. But if you prefer I'll make it secondary like the "Refresh all" button.

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

Yes make it secondary like the refresh btn next to it

@Jor-Tech
Copy link
Author

image

Here you go ser. PR updated.

@Fredolx
Copy link
Owner

Fredolx commented Apr 25, 2025

Everything is perfect except one thing, please remove all your changes from
src/app/whats-new-modal/whats-new-modal.component.html

I only modify this file when I do a release. I'm not ready for a release this week

@Jor-Tech
Copy link
Author

Everything is perfect except one thing, please remove all your changes from src/app/whats-new-modal/whats-new-modal.component.html

I only modify this file when I do a release. I'm not ready for a release this week

Got it. Removed it from the "change log" whats-new-modal.component.html

@Fredolx Fredolx merged commit 2a9bc29 into Fredolx:main Apr 25, 2025
3 checks passed
@Jor-Tech Jor-Tech deleted the feature/clear-history branch April 25, 2025 23:01
@Fredolx
Copy link
Owner

Fredolx commented Apr 26, 2025

Thank you for your contribution. If you are interested in more work, I can open up a kanban board from which you could choose features you'd like to work on. There's a few features that could be implemented/improved. Stalker supports needs to be investigated, and the catchup feature needs to be more stable.

@Jor-Tech
Copy link
Author

Thank you for your contribution. If you are interested in more work, I can open up a kanban board from which you could choose features you'd like to work on. There's a few features that could be implemented/improved. Stalker supports needs to be investigated, and the catchup feature needs to be more stable.

Sure, I can take a look at the kanban board to see if there is anything I can help with and work on.
Just don't expect to much from me, as I'm limited in code knowledge and time to frequently work on things. I'm more of a hobbyist coder, but hey, who knows. This little feature improvement worked out well enough.

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.

3 participants