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

[React branch] Fixes and polish part five #1492

Conversation

clpetersonucf
Copy link
Member

@clpetersonucf clpetersonucf commented Aug 24, 2023

Bug fixes:

  • Fixes URL binding when copying widgets from the instance admin interface
  • Fixes URL hash matching in my-widgets
  • Fixes lack of support for LTI legacy URLs
  • Fixes style bug with widget players not utilizing the vertical space of the viewport when a discrete height is not provided in the install.yaml
  • Updates to useUpdatedWidget mutation
  • Fixes to widget catalog filtering based on metadata
  • Numerous fixes to score screen issues
  • Fixed score screen linking from user admin instance history panel
  • Small behavioral fixes to Preview and Save Draft buttons in the creator action bar

Improvements:

  • Lots of style cleanup
  • Significant overhaul of the Scores component, including splitting logic into child ScoreOverview, ScoreDetails, and ScoreGraphic components
  • Reworked score screen styling to better follow the React branch's design language
  • Improved conditional rendering of score screen error states (invalid, restricted, expired)
  • Improved usability of user admin instance history by providing play date and complete/incomplete in collapsed view
  • Added a "Last saved Xm ago" message to the creator action bar that's displayed when the instance was last saved in-place, via the Save Draft or Preview buttons
  • Tweaked styling of homepage to make it a little cleaner, replaced image assets
  • Adds a user role management interface to the user admin panel that's only available to super users

@clpetersonucf clpetersonucf marked this pull request as ready for review September 13, 2023 21:35
@clpetersonucf
Copy link
Member Author

Ready for review.

Copy link
Contributor

@cayb0rg cayb0rg left a comment

Choose a reason for hiding this comment

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

Looks and works great! This might be a good PR too to push a few quick fixes to other score screen issues, such as the missing Preview/Play again button. There are also a couple useEffects that could be reduced to just setting the enabled attribute on the query itself to the dependency, such as loadPlayScores and possibly loadGuestScores and loadInstanceScores.

setAttributes({
...attributes,
hidePlayAgain: true
})
}
}
}, [attemptsLeft])
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a race condition here with instance and single_id being undefined when attemptsLeft has loaded. Possible fix is to add instance as a dependency.

The check for !single_id also seems unnecessary, since it will still pass even when it is null or undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the sanity check. This got rearranged so that setAttemptsLeft is only assigned when the apiGetWidgetInstanceScores query resolves, which itself is predicated on instance existing.

@clpetersonucf
Copy link
Member Author

clpetersonucf commented Sep 14, 2023

missing Preview/Play again button

@cayb0rg I can't believe I overlooked the missing Preview Again button, but do you have examples of when the Play Again button should be present and isn't?

@cayb0rg
Copy link
Contributor

cayb0rg commented Sep 14, 2023

Play Again is missing when a widget is set to Guest Mode, because attemptsLeft is never set the second time (the first time, instance hasn't loaded yet). In Normal mode, attemptsLeft gets set after the instance scores are loaded.

Adding instance as a dependency to the useEffect for attemptsLeft I think should fix it.

@FrenjaminBanklin
Copy link
Contributor

FrenjaminBanklin commented Sep 14, 2023

The only other things I see are incredibly minor.

  • If beard mode is enabled, beards are not applied to the selected widget's large icon if an instance is auto-loaded in the My Widgets page.
  • If the 'Save Draft' button is clicked and something prevents the widget from saving properly (a Labeling widget does not have at least one label, for example), the 'Save Draft' button reads 'Saving...' and does not change back until a successful save follows an additional click.
    • If the 'Publish' button is clicked, then the subsequence 'Yes, Publish' button is clicked and something prevents the widget from saving properly, the 'Preview' button text is changed to say 'Saving...', and can only be fixed by refreshing the page.

@clpetersonucf
Copy link
Member Author

  • Removed hooks that were based on guestAccess, previewInstId, and playId being assigned. The functionality associated with guestAccess has been condensed into the hook waiting for instance to be set and available. The query for apiGetWidgetInstancePlayScores has been updated to trigger when playId or previewInstId are set, removing the need for those hooks.
  • Fixed "Play/Preview" again options not being available for preview and guest plays.
  • The score summary graph is no longer displayed for score screens where single_id is set.

@clpetersonucf
Copy link
Member Author

  • Added a state variable in the creator that only updates when a save request is made, and deferred creator saveState updates to a hook tracking the new variable. This should hopefully resolve race conditions when the API response returns before the prior creatorState update is complete.
  • Removed Preview button text updating when clicked, since it was kinda confusing and clarity was added via
  • Added a message to the action bar that displays after the instance is saved (via Save Draft or Preview) showing the duration in minutes since the last save was made.

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.

I can see the widget catalog filter options still being kind of a hassle to figure out for unsighted users, but I don't really have any ideas for how to fix it and everything within the scope of this PR looks good.

…nager

Adds user role manager to user admin interface
@clpetersonucf clpetersonucf merged commit 1d8ffbf into ucfopen:issue/support-dashboard-in-react Sep 21, 2023
2 checks passed
@clpetersonucf clpetersonucf deleted the react/fixes-and-polish-part-five branch September 21, 2023 14:29
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