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

  • Bug Fixes
    • Updated UI timing: a consistent baseline delay is now applied when target elements are missing. When elements are present, that baseline is added before any transition delays, increasing total wait time in cases with non-zero transitions and removing the previous longer fallback. This changes the timing of subsequent UI refreshes.

@github-actions github-actions bot added bug Something isn't working release merge to release/ branch, before release period labels Oct 31, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

getElementDurationTime now uses a 50ms baseline delay. If the target element is missing it returns 50ms immediately; if present, it starts at 50ms and adds computed CSS transition durations and delays to that baseline.

Changes

Cohort / File(s) Summary
Timing adjustment for element duration calculation
packages/canvas/container/src/container.ts
getElementDurationTime initializes delayTime at 50ms (was 0), returns 50ms early when element missing, and accumulates computed transition durations/delays on top of the 50ms baseline (removed previous 300ms fallback).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review packages/canvas/container/src/container.ts for intended early-return behavior when element is missing.
  • Validate that adding a 50ms baseline and removing the 300ms fallback doesn't break timing assumptions in callers (selection/rect updates).
  • Check unit tests or UI flows that rely on specific delay values.

Poem

🐰 Fifty ticks beneath my paw,

a tiny pause before we draw.
Baseline set and stacks accrue,
I hop, I wait — then paint anew.
A softer beat for code and crew.

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 pull request title "Fix/element not found v2.8:Add condition for when element is null" directly and specifically addresses the primary change described in the raw summary and PR objectives. The main modification is in the getElementDurationTime function to handle the case when a target element is missing by initializing and returning a default delay of 50ms. The title accurately captures this core fix by explicitly referencing "element not found" and "when element is null," which are the exact conditions being addressed. The title is concise, clear, and provides sufficient context without excessive detail or vague terminology.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e57bfa and 624204e.

📒 Files selected for processing (1)
  • packages/canvas/container/src/container.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T06:55:14.457Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-01-14T06:55:14.457Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T06:50:21.158Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/data-function/parser.ts:191-195
Timestamp: 2025-01-14T06:50:21.158Z
Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.

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

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 v2.8: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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/canvas/container/src/container.ts (1)

497-497: Unreachable fallback: delayTime can never be 0.

Since delayTime is now initialized to 50 (line 471) and the function returns early if no element is found (line 473), the condition delayTime === 0 at line 497 is unreachable. The fallback to 300ms will never execute.

Consider simplifying line 497 to just return delayTime:

-  return delayTime === 0 ? 300 : delayTime
+  return delayTime
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de9ef61 and 1e57bfa.

📒 Files selected for processing (1)
  • packages/canvas/container/src/container.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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
📚 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-14T06:55:14.457Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-01-14T06:55:14.457Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T06:59:02.999Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/material-function/support-collection.ts:3-15
Timestamp: 2025-01-14T06:59:02.999Z
Learning: The code in `packages/canvas/render/src/material-function/support-collection.ts` is migrated code that should not be modified at this time to maintain stability during the migration process.

Applied to files:

  • packages/canvas/container/src/container.ts
📚 Learning: 2025-01-14T07:11:44.138Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/canvas-function/custom-renderer.ts:28-32
Timestamp: 2025-01-14T07:11:44.138Z
Learning: The locale and webComponent wrapper in `packages/canvas/render/src/canvas-function/custom-renderer.ts` are part of migrated code and will be improved in a future PR.

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

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

Labels

bug Something isn't working release merge to release/ branch, before release period

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant