Extract reusable warning banner and link planning hints - #140
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfea75d3bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| const openLearningTimes = () => { | ||
| router.push(ROUTES.learningTimes); |
There was a problem hiding this comment.
When this CTA is tapped from the generated-plan review, it pushes /learning-times; I checked src/app/learning-times/index.tsx and its header back handler routes to /settings whenever router.canGoBack() is true, so users who follow the planning hint to adjust learning times cannot return to the draft plan via the normal back control and can get dropped into the settings stack instead. Preserve a return target for this new entry point or make the learning-times back path return to the originating plan screen.
Useful? React with 👍 / 👎.
|
Please add Screenshots next time |
|
Please solve merge conflicts |
Gamius00
left a comment
Gamius00
left a comment
There was a problem hiding this comment.
Merge Conflicts, Add Screenshots to PR
|
Fixed and pushed PR #140 merge conflicts. Commit pushed: Conflicts
Also updated warning-banner.tsx to use the release design tokens/classes, and preserved the learning-times Validation passed: |
|
Please only merge after merging #145 |
…reusable-warning-link # Conflicts: # src/app/learning-plans/[planId]/index.tsx # src/app/learning-plans/[planId]/review.tsx # src/app/notifications.tsx
them locally we can do that via a .tmp folder or something and for everything else they are already saved inside the PR. No need to have them twice (duplicated). Agents should upload them directly via their plugins etc.
|
Please only merge after merging: #147 |
2 participants
What changed
Dayova now turns passive learning-plan warnings into actionable guidance. When a generated plan reports missing or constrained Lernzeiten, the planning hint uses the shared warning banner and can send the user directly to the Lernzeiten screen while preserving a return path back to the plan.
Why
Before this PR, users could be told that no usable learning times were configured, but the warning itself did not give them a way to fix it. This makes the recovery path obvious and reuses the same warning/CTA component already needed by the notifications inbox.
Summary
WarningBannercomponent for warning + CTA messaging.PlanningHintBanner.learningTimesroute constant plusreturnTohelpers so the Lernzeiten overview/editor can keep the user in the learning-plan flow.Screenshots
Captured on the iOS simulator, iPhone 17 Pro.
Learning-plan warning placement
learning-plans/[planId]/review.tsxlearning-plans/[planId]/index.tsxRelated warning and Lernzeiten flow
Testing
fnm exec --using=v22.23.0 ./node_modules/.bin/biome format .fnm exec --using=v22.23.0 ./node_modules/.bin/biome lint . --only=nursery/useSortedClassesfnm exec --using=v22.23.0 ./node_modules/.bin/tsc --noEmitfnm exec --using=v22.23.0 ./node_modules/.bin/eslint src convex index.ts