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.

Sign up for GitHub

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

Jump to bottom

[React branch] Widget player component refactor #1497

Merged
merged 9 commits into from
Sep 29, 2023
Merged

[React branch] Widget player component refactor #1497

merged 9 commits into from
Sep 29, 2023

Conversation

clpetersonucf
Copy link
Member

@clpetersonucf clpetersonucf commented Sep 26, 2023 •
edited
Loading

  • Heavy refactor of the widget player component, to address potential race conditions with logging and streamline various bits of the logic associated with handling, enqueuing, and submitting play logs.
  • Fixed improper type matching of the is_valid property of Session_Play
  • Implemented missing session_play_verify heartbeat associated with active widget plays

@clpetersonucf clpetersonucf marked this pull request as ready for review September 26, 2023 18:06
FrenjaminBanklin
FrenjaminBanklin approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

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

It looks like everything's working here.

There are some potential super minor nitpicks mentioned below but nothing that should prevent this from moving forward.

fuel/app/classes/materia/api/v1.php Outdated Show resolved Hide resolved
src/components/widget-player.jsx Outdated Show resolved Hide resolved
src/components/widget-player.jsx Outdated Show resolved Hide resolved
@clpetersonucf clpetersonucf merged commit 84fc790 into ucfopen:issue/support-dashboard-in-react Sep 29, 2023
2 checks passed
@clpetersonucf clpetersonucf deleted the react/widget-player-component-refactor branch September 29, 2023 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrenjaminBanklin FrenjaminBanklin FrenjaminBanklin approved these changes

@cayb0rg cayb0rg cayb0rg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clpetersonucf @FrenjaminBanklin @cayb0rg