Skip to content

feat(tutorial): refine editor-leave confirmation and course success modal navigation#3305

Open
Ethanlita wants to merge 5 commits into
goplus:devfrom
Ethanlita:feat/tutorial-leave-and-series-nav
Open

feat(tutorial): refine editor-leave confirmation and course success modal navigation#3305
Ethanlita wants to merge 5 commits into
goplus:devfrom
Ethanlita:feat/tutorial-leave-and-series-nav

Conversation

@Ethanlita

@Ethanlita Ethanlita commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduce an editor-owned extension point for the "leave editor" confirmation and use it to refine the tutorial experience, so tutorial features can adjust the editor's leave behavior .

User-facing changes

  • Tutorial-aware leave confirmation. While a tutorial course is active, the "Leave editor" confirmation explains that tutorial edits will not be saved; outside tutorials it keeps the existing behavior.
  • No redundant confirmation for expected course navigation. Explicit, expected navigations away from the editor via the course success modal no longer pop the leave confirmation:
    • continuing to the next course, and
    • returning to the course series from the success modal.
  • Course success modal links back to the series. The success modal's secondary action is "Back to series courses", which opens the current course's series page (/course-series/:id). ("Learn next course" is unchanged.)

Implementation & design

  • New editor-owned controller components/editor/leave-confirm.ts (editorLeaveConfirm) — a generic extension point for the editor's leave confirmation:
    • messageOverride — replace the confirmation copy.
    • requestSkipOnce / consumeSkipOnce — skip the confirmation for a single, expected navigation. The skip is time-bound, so a request that is never consumed (e.g. an editor → editor route update, or starting a course from a non-editor page, neither of which reaches onBeforeRouteLeave) cannot get stuck and silently suppress a later, genuine leave.
  • The editor stays unaware of tutorials. The editor page's onBeforeRouteLeave guard only reads editorLeaveConfirm. The tutorial layer drives it, the same way it drives copilot: TutorialRoot sets the tutorial copy while a course is active (and clears it on cleanup), and startCourse / the success modal request the skip. There is no components/editor → components/tutorials dependency, and pages/editor no longer imports useTutorial.

Validation

  • npm run type-check, npm run lint, and editing.test.ts (22 tests) pass.

Ethanlita and others added 2 commits June 23, 2026 18:17
…eries

- Show tutorial-specific copy in the leave-editor confirm dialog when a
  course is active; keep the original copy otherwise
- Make the course success modal's "back to course series" button navigate
  to the current course's series instead of /tutorials

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a one-shot skip flag on Tutorial so explicit, expected navigations away
from the editor do not trigger the leave confirmation:

- "Back to course series" in the success modal requests the skip before
  navigating to the series page
- startCourse requests the skip before pushing the course entrypoint, which
  may point to a non-editor route

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to skip the editor's leave confirmation dialog during explicit, expected navigation actions (such as starting a new course or returning to the course series page) and updates the confirmation message to be tutorial-specific when in a tutorial. Feedback was provided regarding a potential bug where the skip-confirmation flag could get "stuck" as true if navigating between editor routes (which triggers onBeforeRouteUpdate instead of onBeforeRouteLeave). A time-bound expiration mechanism was suggested to resolve this issue.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread spx-gui/src/components/tutorials/tutorial.ts Outdated
A plain one-shot flag could leak: navigations that don't reach the editor's
onBeforeRouteLeave (e.g. editor -> editor route updates when starting the next
course, or starting a course from a non-editor page) never consume it, so a
later genuine leave would silently skip its confirmation. Expire the skip
request after skipLeaveConfirmTimeout instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

The time-bounded skip-flag design is solid — the comments clearly explain both what the mechanism does and why expiry is necessary. The correctness tradeoff (a skip request that is never consumed can suppress a genuine leave within its 1 s window) is explicitly acknowledged in the PR commit messages, which is good. A few issues worth addressing before merge:

Ordering risk in handleBackToCourseSeriesrequestSkipLeaveConfirm() is called after emit('cancelled'). If the parent's cancelled handler takes any time, the 1 s window starts later than necessary. Moving requestSkipLeaveConfirm() to be the first call in the function minimises the gap and removes dependence on the parent handler's cost:

function handleBackToCourseSeries() {
  props.tutorial.requestSkipLeaveConfirm() // first
  emit('cancelled')
  router.push(`/course-series/${props.series.id}`)
}

Unhandled router.push rejectionrouter.push(...) returns a promise that can reject (navigation guard cancellation, NavigationDuplicated, etc.). handleStartNextCourse is already wrapped in useMessageHandle; it's worth adding a .catch or similar error handling to handleBackToCourseSeries as well, consistent with the surrounding code.

Button label capitalisation'Back to Series Courses' uses Title Case, while every other button in this file and neighbouring pages uses sentence case ('Learn next course', 'Back to apps', 'Back to users'). Should be 'Back to series courses'.

Comment/label mismatchtutorial.ts line ~73 quotes the button as "back to course series", but the actual label is "Back to Series Courses" (word order differs too). Should match the rendered string.

zh copy inconsistency — The tutorial and project leave-confirm messages use different confirmation verbs: 确认要离开吗 vs 确定要离开吗. Aligning them avoids a subtle inconsistency in the UX.

Comment thread spx-gui/src/components/tutorials/TutorialCourseSuccessModal.vue Outdated
Comment thread spx-gui/src/components/tutorials/TutorialCourseSuccessModal.vue Outdated
Comment thread spx-gui/src/components/tutorials/TutorialCourseSuccessModal.vue Outdated
Comment thread spx-gui/src/components/tutorials/tutorial.ts Outdated
Comment thread spx-gui/src/apps/xbuilder/pages/editor/index.vue Outdated
- Request the leave-confirm skip before emit('cancelled') so the time-bound
  window isn't shortened by the cancel handler
- Wrap "back to course series" navigation in useMessageHandle to surface
  router.push rejections, consistent with handleStartNextCourse
- Use sentence case for the button label: "Back to series courses"
- Align the tutorial.ts comment with the actual button label

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Invert the dependency so the editor no longer needs tutorial knowledge.
Add an editor-owned `editorLeaveConfirm` controller exposing a generic
message override and a one-shot, time-bound skip. Tutorial drives it the
same way it drives copilot (TutorialRoot sets the tutorial-specific copy
while a course is active; startCourse and the success modal request the
skip), and the editor page's leave guard only reads the controller.

The editor stays unaware of tutorials (no components/editor -> tutorials
import), and pages/editor no longer imports useTutorial.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
// A course entrypoint may point to a non-editor route. Navigating away from the
// editor would otherwise trigger its leave confirmation, but starting a course is
// an explicit, expected action, so skip the confirmation for this navigation.
editorLeaveConfirm.requestSkipOnce()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

跳过 confirm 的逻辑放到 TutorialCourseSuccessModal.vue 中调用 startCourse 的地方做可能更合适?按我们讨论的结论,不是所有的 startCourse 都应该跳过 confirm 的,只是 TutorialCourseSuccessModal 中那个 startCourse 行为才应该跳过

* If it is OK to leave, return true, otherwise return false.
*/
async function checkChangesNotToBeSaved(es: EditorState) {
if (editorLeaveConfirm.consumeSkipOnce()) return true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

通过单独的机制来避免跟 editor 跟 tutorial 逻辑的耦合,这个思路我觉得没问题。不过现在这个 editor-leave-confirm 稍微有点重,我在想能不能借用已有的 editing-dirty 来达到这次的目的;比如 class Editing 增加个方法来重置 dirty 状态,现在 requestSkipOnce 的地方通过重置 dirty 状态来跳过检查?

// Drive the editor's leave confirmation with tutorial-specific copy while a course is
// active; the editor exposes the override and stays unaware of tutorials.
editorLeaveConfirm.setMessageOverride({
en: 'Tutorial edits will not be saved if you leave now. Are you sure to leave?',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这个文案跟默认文案表达的信息差别并不大,有没有可能优化默认文案来让它在不同场景下都比较合适,而不是针对 tutorial 使用不一样的文案?

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