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

random questions #296

Merged
merged 1 commit into from
Aug 6, 2023
Merged

random questions #296

merged 1 commit into from
Aug 6, 2023

Conversation

mruwnik
Copy link
Collaborator

@mruwnik mruwnik commented Aug 3, 2023

@mruwnik
Copy link
Collaborator Author

mruwnik commented Aug 5, 2023

ping

Copy link
Collaborator

@ccstan99 ccstan99 left a comment

Choose a reason for hiding this comment

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

LGTM! Don't have time to test it out but I know Rob/plex want this as a priority. @Aprillion feel free to add anything else you feel relevant.

@mruwnik mruwnik merged commit 189f6cd into master Aug 6, 2023
1 check passed
@mruwnik mruwnik deleted the random-question branch August 6, 2023 09:57
Copy link
Collaborator

@Aprillion Aprillion 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 for initial implementation

FTR there is usually no point pinging me on the same platform, I'm not checking emails that much during vacation 😅 I would have noticed https://discord.com/channels/677546901339504640/1125882087908573305 sooner if needed next time (though also not 100%)

questionState: QuestionState.OPEN,
}
} catch (e) {
console.error(e)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the deal with the server side logging here, please? does it do anything different than letting the exception unhandled?

why not send the error to client a la https://github.com/StampyAI/stampy-ui/blob/master/app/routes/questions/%24question.tsx ?

<Question
questionProps={question}
onLazyLoadQuestion={onLazyLoadQuestion}
onToggle={toggleQuestion}
Copy link
Collaborator

Choose a reason for hiding this comment

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

toggle does not work for me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

doesn't work for me, either. Though it should be fine? It would be nice for this to also handle related questions and the search bar, but it didn't seem worth the bother...

Copy link
Collaborator

Choose a reason for hiding this comment

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

would be even better if there was no > chevron if the feature doesn't make sense .. which it probably doesn't 😅

a refresh button at the bottom (similar to load more questions) saying Reload for a another random question might also be nice some time in the future

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