Skip to content

Conversation

@betterdancing
Copy link
Contributor

@betterdancing betterdancing commented Oct 31, 2025

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

elementId存在但是找不到对应的element元素,例如teleport 或者是 顶层是 fragment 的元素
Issue Number: N/A

What is the new behavior?

如果在elementId找不到对应的元素,则直接return默认的delay毫秒50ms

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Chores
    • Refined internal code patterns for consistency across workspace modules and container utilities.

@github-actions github-actions bot added the bug Something isn't working label Oct 31, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

The changes include a behavioral modification to element duration time calculation logic and stylistic refactoring of filter predicates across two Vue components. The container module now defaults to a 50ms delay and short-circuits when elements are not found, while application and template center modules adopt the more explicit Boolean() function for truthiness checks.

Changes

Cohort / File(s) Summary
Element Duration Logic
packages/canvas/container/src/container.ts
Modified getElementDurationTime to initialize delayTime to 50 instead of 0 and return early when the targeted element is not found, changing the computed wait time behavior for missing elements.
Filter Predicate Refactoring
packages/workspace/application-center/src/Main.vue, packages/workspace/template-center/src/Main.vue
Updated filter predicates in getApplicationList and getTemplateList from !!value to Boolean(value) for functionally equivalent but more explicit truthiness checking.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The container.ts logic change introduces a new default value (50) and early-return behavior—verify that the 50ms default is appropriate for the missing-element use case and that the short-circuit doesn't skip necessary fallback logic.
  • The Vue component changes are straightforward cosmetic refactoring with no behavioral impact.

Poem

🐰 A bunny hops through code so neat,
With fifty milliseconds sweet,
And Boolean makes truth clear,
No more double-bang here!
The logic flows, and tests retreat.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix/element not found develop:Add condition for when element is null" is clearly related to the primary change in this changeset. The main modification in packages/canvas/container/src/container.ts adds a condition to return a default delay of 50 ms when an element cannot be resolved (element is null), which directly aligns with the title's stated purpose. The two secondary changes to the filter predicates in the Vue files are minor stylistic refactorings that don't need to be captured in the title. A teammate reviewing the commit history would understand that this PR addresses missing element handling with a default timeout.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@betterdancing betterdancing changed the title Fix/element not found:Add condition for when element is null Fix/element not found develop:Add condition for when element is null Oct 31, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/workspace/template-center/src/Main.vue (1)

173-173: Stylistic change to use Boolean() function.

The change from !!value to Boolean(value) is functionally equivalent—both filter out falsy values. This is purely a stylistic preference for explicitness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9147a71 and 8c19cef.

📒 Files selected for processing (3)
  • packages/canvas/container/src/container.ts (1 hunks)
  • packages/workspace/application-center/src/Main.vue (1 hunks)
  • packages/workspace/template-center/src/Main.vue (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gene9831
Repo: opentiny/tiny-engine PR: 1226
File: packages/canvas/container/src/components/CanvasDivider.vue:184-185
Timestamp: 2025-03-19T03:13:51.520Z
Learning: The CSS bug in packages/canvas/container/src/components/CanvasDivider.vue where verLeft already includes "px" but is being appended again in the style object will be fixed in a future update, as confirmed by gene9831.
📚 Learning: 2025-01-14T08:45:57.032Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/application-function/global-state.ts:12-25
Timestamp: 2025-01-14T08:45:57.032Z
Learning: The code in `packages/canvas/render/src/application-function/global-state.ts` is migrated from an existing codebase and should be handled with care when making modifications.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T08:44:09.485Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/canvas-function/controller.ts:1-7
Timestamp: 2025-01-14T08:44:09.485Z
Learning: Type safety improvements for the controller in `packages/canvas/render/src/canvas-function/controller.ts` should be deferred until the data structure is finalized.

Applied to files:

  • packages/canvas/container/src/container.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: push-check
🔇 Additional comments (2)
packages/workspace/application-center/src/Main.vue (1)

194-199: LGTM - Consistent stylistic improvement.

The change from !!value to Boolean(value) mirrors the same refactor in template-center/src/Main.vue, providing consistency across the codebase. Functionally equivalent.

packages/canvas/container/src/container.ts (1)

471-475: Good defensive fix for missing elements.

Initializing delayTime to 50ms and returning early when the element is null prevents a crash when calling getComputedStyle on a non-existent element. This properly handles cases where an elementId exists but the corresponding DOM element cannot be found (e.g., teleport or fragment components).

The 50ms default is reasonable for transition timing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant