Experiment: Stage 1 - Show All Steps At Once #2504
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
andy1li
force-pushed
the
stage-1-experiments
branch
from
0d45ebd to
b3f4200
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs (1)
61-76: “Tests passed” vs. “Tests failed” messaging
The strikethrough usage is a neat way to show progression. Make sure the fallback text is comprehensible if test statuses are in an unexpected state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs(1 hunks)app/components/course-page/course-stage-step/first-stage-tutorial-card.ts(3 hunks)app/services/feature-flags.js(1 hunks)app/templates/course/stage/instructions.hbs(1 hunks)
🔇 Additional comments (15)
app/services/feature-flags.js (1)
13-15: Validate staff check vs. feature flag logic
The new canSeeAllStepsAtOnceForStage1 getter uses the currentUser?.isStaff check or a 'test' flag value to allow visibility. Ensure that this aligns with the intended product requirements and that 'test' is the only permissible override.
app/components/course-page/course-stage-step/first-stage-tutorial-card.ts (5)
9-9: Good addition of CourseStageStep import
Importing CourseStageStep clarifies the type for currentStep. This addition improves code readability and type safety.
19-19: Check for potential null usage
Be certain that currentStep is always passed into this component. If it's undefined, calls to this.args.currentStep could be problematic.
76-78: Straightforward getter implementation
currentCourse is a concise getter referencing courseStage.course. Looks good.
80-82: Clarify condition in hasPassedTests
this.args.currentStep.testsStatus === 'passed' or this.args.currentStep.status === 'complete' is a clear check. Verify that these statuses are consistently set in upstream logic so that the getter remains accurate.
92-94: Use feature flags for dynamic step rendering
shouldShowAllStepsAtOnceForStage1 delegating to featureFlags.canSeeAllStepsAtOnceForStage1 is a solid approach for toggling UI logic. Confirm that the feature flag is consistently set for relevant users in production/staging.
app/templates/course/stage/instructions.hbs (1)
36-41: Passing @currentStep is beneficial
Adding @currentStep ensures the component has all relevant data, especially for multi-step logic. Great addition for clarity.
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs (8)
1-10: Conditional UI for showing all steps
When this.shouldShowAllStepsAtOnceForStage1 is true, the new InstructionsCard block enumerates steps directly. This is a clear, user-friendly alternative, but ensure you’ve tested both paths (flag on/off).
11-26: Step 1 structure
Nicely separates “Open a file” into its own subcomponent NavigateToFileStep. This helps maintain step-by-step clarity.
27-44: Step 2 structure
Both the text and the dedicated UncommentCodeStep subcomponent highlight the user’s next action. This streamline is good.
45-60: Step 3 includes instructions for submitting code
Clear instructions are given with a sample Git command. This is intuitive for new users. The usage of CopyableTerminalCommand adds convenience.
77-96: Encourage forum help
Linking directly to a new forum topic is a user-friendly mechanism. Everything here looks straightforward.
100-127: Fallback to ExpandableStepList
Retaining the original ExpandableStepList path ensures backward compatibility if the feature flag is off. Good approach.
128-141: “Tests failed” note
This note clarifies the user journey during the first Git push. The short explanation is likely helpful to newcomers.
142-144: Closing conditional
Ends the InstructionsCard gracefully. No issues noted.
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs
Outdated
Show resolved
Hide resolved
app/components/course-page/course-stage-step/first-stage-tutorial-card.hbs
Outdated
Show resolved
Hide resolved
…first stage tutorial card and instructions page
|
…1ForumLinkCTA` and update its logic Updated the name of the `shouldShowNeedHelpForumLink` property to `shouldShowStage1ForumLinkCTA` and modified its logic to align with its new name.
Checklist:
[percy]in the message to trigger)Summary by CodeRabbit
New Features
Bug Fixes
Documentation