Skip to content

feat: adds assignment flow #1627

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: adds assignment flow #1627

wants to merge 2 commits into from

Conversation

katrinan029
Copy link
Contributor

@katrinan029 katrinan029 commented Jul 24, 2025

For all changes

https://2u-internal.atlassian.net/browse/ENT-10521

Screen.Recording.2025-07-23.at.11.14.28.PM.mov
  • Ensure adequate tests are in place (or reviewed existing tests cover changes)

Only if submitting a visual change

  • Ensure to attach screenshots
  • Ensure to have UX team confirm screenshots

@katrinan029 katrinan029 requested a review from a team July 29, 2025 15:36
Copy link
Contributor

@marlonkeating marlonkeating left a comment

Choose a reason for hiding this comment

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

Seeing some issues when

Orphaned highlight around button when navigating into budget
https://github.com/user-attachments/assets/d366a949-66e4-4817-a9c0-11d0b49a7de9

Not scrolling far enough when transitioning from step 6-7
https://github.com/user-attachments/assets/902b2acc-0afd-4b4d-b79c-4f39ab5e08c1

export const ALLOCATE_LEARNING_BUDGETS_TARGETS = {
SIDEBAR: 'learner-credit-link',
VIEW_BUDGET: 'learner-credit-view-budget-button',
ASSIGNMENT_BUDGET_DETAIL_CARD: 'assignment-budget-detail-card',
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this value hardcoded in a lot of places in the code, can we use the constant here?

@@ -59,7 +88,14 @@ const AdminOnboardingTours: FC<AdminOnboardingToursProps> = ({
if (adminOnboardingSteps[currentStep]) {
const nextTarget = adminOnboardingSteps[currentStep].target;
const targetWithoutPrefix = nextTarget.replace(/^[.#]/, '');
setTarget(targetWithoutPrefix);
// Add timeout for assignment-budget-detail-card to wait for page loading
if (targetWithoutPrefix === 'assignment-budget-detail-card') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a more responsive and reusable way we could do this, if we really need to wait for elements to load before making the switch? Especially since we're doing the same wait in CheckpointOverlay.

Copy link

codecov bot commented Jul 30, 2025

Codecov Report

❌ Patch coverage is 74.07407% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.78%. Comparing base (3b8716f) to head (e3ccd74).

Files with missing lines Patch % Lines
...AdminOnboardingTours/flows/AdminOnboardingTour.tsx 55.00% 9 Missing ⚠️
...ours/AdminOnboardingTours/AdminOnboardingTours.tsx 60.00% 2 Missing ⚠️
src/components/ProductTours/ProductTours.jsx 0.00% 2 Missing ⚠️
src/components/ProductTours/CheckpointOverlay.tsx 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1627      +/-   ##
==========================================
- Coverage   86.82%   86.78%   -0.05%     
==========================================
  Files         779      780       +1     
  Lines       17634    17683      +49     
  Branches     3670     3694      +24     
==========================================
+ Hits        15311    15346      +35     
- Misses       2247     2270      +23     
+ Partials       76       67       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants