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] Update getPlaystreakQuestStatus to hide if ineligible completed quest #40

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

BrettCleary
Copy link
Contributor

@BrettCleary BrettCleary commented Dec 6, 2024

@BrettCleary BrettCleary self-assigned this Dec 6, 2024
Copy link

vercel bot commented Dec 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
quests-ui ✅ Ready (Inspect) Visit Preview Dec 8, 2024 7:16am

@@ -23,6 +23,8 @@ export function getPlaystreakQuestStatus(
(gameIsInfinitelyCompleteable || gameHasMoreFiniteCompletionsPossible)
) {
return 'READY_FOR_CLAIM'
} else if (quest.status === 'COMPLETED') {
return undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

Why undefined instead of addind another option to the type? I find a value like NOT_QUALIFIED better than the absence of value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want it to be confused with a QuestLogInfo['state'] value. undefined in this case means the quest shouldn't be shown to the user.

I do see where your coming from and would make the change if I weren't under a bit of a time crunch. This is a bit of a nitpick though

@BrettCleary BrettCleary merged commit 309bed4 into main Dec 18, 2024
2 checks passed
@BrettCleary BrettCleary deleted the feat/hide_ineligible_completed_quests branch December 18, 2024 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants