feat(embedded): add Iframe class for <iframe> element.#95
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a new ChangesIframe Class and Test Coverage
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@codex review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 518 529 +11
===========================================
Files 96 97 +1
Lines 1159 1183 +24
===========================================
+ Hits 1159 1183 +24 ☔ View full report in Codecov by Sentry. |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Embedded/IframeTest.php`:
- Around line 747-773: Add explicit serialization tests for the width()
attribute in the Iframe tests by mirroring the existing src/srcdoc tests: in the
IframeTest add a new test method (e.g., testRenderWithWidth) that calls
Iframe::tag()->width('123')->render() and asserts the output contains <iframe
width="123">...</iframe> using self::assertSame with the same heredoc style used
for testRenderWithSrc and testRenderWithSrcdoc; ensure you reference the
Iframe::tag() and width() methods so the test fails if attribute mapping is
incorrect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1a874cc-8a06-4a0c-8f5e-289e8debd7ee
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdscaffold-lock.jsonsrc/Embedded/Iframe.phptests/Embedded/IframeTest.php
📜 Review details
⏰ 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). (18)
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: easy-coding-standard / PHP 8.5-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.5-ubuntu-latest
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2026-02-22T19:26:33.769Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Sectioning/AsideTest.php:821-833
Timestamp: 2026-02-22T19:26:33.769Z
Learning: In the ui-awesome/html repository, do not use import aliases in PHP. When there are naming conflicts (e.g., two different Message classes), prefer the fully qualified class name (FQCN) such as \UIAwesome\Html\Attribute\Exception\Message::ATTRIBUTE_INVALID_VALUE instead of creating an alias like "use UIAwesome\Html\Attribute\Exception\Message as AttributeMessage". This should apply to PHP files across the codebase (not just tests).
Applied to files:
src/Embedded/Iframe.phptests/Embedded/IframeTest.php
📚 Learning: 2026-03-09T22:08:26.569Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 79
File: src/Table/Tr.php:44-50
Timestamp: 2026-03-09T22:08:26.569Z
Learning: In the ui-awesome/html repository, allow short PHP variable names (e.g., $tr, $id) in all PHP files and do not apply PHPMD ShortVariable checks to PHP files. This applies to both production source and tests. If PHPMD is configured, adjust or suppress the ShortVariable rule globally for PHP files to reflect this policy.
Applied to files:
src/Embedded/Iframe.phptests/Embedded/IframeTest.php
📚 Learning: 2026-03-08T18:01:07.256Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 73
File: src/Form/Optgroup.php:40-42
Timestamp: 2026-03-08T18:01:07.256Z
Learning: In PHP code within the ui-awesome/html repo, all HTML tag classes implement Stringable. Passing a tag object to content() implicitly casts it to a string, so do not require an explicit ->render() call. Docblock examples may use content(SomeTag::tag()->...) to illustrate the idiomatic Stringable path; do not flag these as stale or replace with helper methods like option(), td(), etc. When reviewing code in PHP files, favor relying on Stringable/string casting for tag objects passed to content() and avoid suggesting a render() call unless there is a concrete, justified need.
Applied to files:
src/Embedded/Iframe.php
📚 Learning: 2026-02-06T21:37:44.509Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 24
File: tests/Form/InputTextTest.php:33-620
Timestamp: 2026-02-06T21:37:44.509Z
Learning: In the ui-awesome/html repository, prefer individual test methods over PHPUnit data providers in test classes. This helps maintainers see exactly how each test behaves without cross-referencing data providers. Apply to all PHP test files under tests. Use separate, descriptively named test methods to cover different inputs/edge cases; retain data providers only if they clearly improve readability or reduce duplication.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-02-11T14:56:18.277Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 37
File: tests/Form/InputTextTest.php:448-457
Timestamp: 2026-02-11T14:56:18.277Z
Learning: In PHP test files under the repository ui-awesome/html (e.g., tests/Form/InputTextTest.php), it is acceptable to use short variable names (e.g., 2-character names like $id). PHPMD's short-variable rule does not need to be strictly followed in test code. During reviews, allow short names in test files and note that this relaxed style applies to tests, while production code should adhere to the standard variable naming conventions.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-02-22T19:37:55.152Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 58
File: tests/Form/InputSubmitTest.php:995-1077
Timestamp: 2026-02-22T19:37:55.152Z
Learning: In PHP test suites for the ui-awesome/html repository, add explicit tests in each element's test file to validate InvalidArgumentException for trait-based attributes (e.g., HasDir, HasLang, HasRole, HasTabIndex, HasTranslate, HasType, etc.). Even if these traits are covered in the html-attribute package, ensure coverage is visible in the element tests because test knowledge may be lost when relying solely on trait package tests. Treat this as a guideline that should be applied broadly across element tests to confirm correct behavior and improve maintainability.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-03-12T12:31:31.417Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 87
File: tests/Embedded/SourceTest.php:200-208
Timestamp: 2026-03-12T12:31:31.417Z
Learning: In tests, prefer using the constructor-array pattern (e.g., Source::tag(['class' => 'default-class']) or ElementClass::tag([...])) to verify default configuration values. These tests are distinct from tests using fluent setters (->class('value')). Ensure tests named testRenderWithDefaultConfigurationValues employ this constructor-array pattern and do not get flagged as overlapping with fluent-setter tests.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-02-21T01:17:40.877Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 51
File: tests/List/UlTest.php:282-283
Timestamp: 2026-02-21T01:17:40.877Z
Learning: In the ui-awesome/html repository, for PHP tests, ensure that assertion messages in tests/List/UlTest.php for methods named testRenderWith*UsingEnum remain identical to the corresponding non-enum tests (i.e., do not append 'using enum' to the failure message). The reason: the underlying render method treats enum and string inputs the same, so failure output should be uniform. Apply this pattern to all similar test cases under tests/ to maintain consistent failure messages across enum vs non-enum variants.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-05-07T21:25:19.358Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 91
File: tests/Phrasing/MarkTest.php:301-320
Timestamp: 2026-05-07T21:25:19.358Z
Learning: In ui-awesome/html PHPUnit test files under tests/, do not require a try/finally guard solely to ensure cleanup around calls to SimpleFactory::setDefaults(). The repository’s convention is that if assertions throw/fail, CI will already fail, so omitting try/finally for SimpleFactory::setDefaults cleanup in these tests should not be flagged.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-05-07T21:25:27.973Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 91
File: tests/Phrasing/EmTest.php:301-320
Timestamp: 2026-05-07T21:25:27.973Z
Learning: In this repository (ui-awesome/html), when writing/updating PHPUnit tests, do not require wrapping `SimpleFactory::setDefaults()` calls in `try/finally` for cleanup of global factory state. If a test fails, CI should fail for the overall run, so explicit `finally`-based state cleanup around `SimpleFactory::setDefaults()` is considered unnecessary overhead—reviewers should not flag the absence of `try/finally` for this specific pattern.
Applied to files:
tests/Embedded/IframeTest.php
📚 Learning: 2026-05-07T21:25:43.766Z
Learnt from: terabytesoftw
Repo: ui-awesome/html PR: 91
File: tests/Flow/PreTest.php:413-433
Timestamp: 2026-05-07T21:25:43.766Z
Learning: In the ui-awesome/html repository, when a test method performs SimpleFactory cleanup via `SimpleFactory::setDefaults(ElementClass::class, [])`, it is acceptable to do this at the end of the method without wrapping the cleanup/assertions in a `try/finally`. Do not raise a review warning for missing `try/finally` solely to ensure cleanup after failed assertions, since failing assertions already fail CI and the additional defensive pattern is not required for this repo.
Applied to files:
tests/Embedded/IframeTest.php
🪛 PHPMD (2.15.0)
src/Embedded/Iframe.php
[error] 114-114: Avoid using static access to class '\UIAwesome\Html\Helper\Validator' in method 'loading'. (undefined)
(StaticAccess)
[error] 114-114: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Loading' in method 'loading'. (undefined)
(StaticAccess)
[error] 158-158: Avoid using static access to class '\UIAwesome\Html\Helper\Validator' in method 'referrerpolicy'. (undefined)
(StaticAccess)
[error] 158-158: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Referrerpolicy' in method 'referrerpolicy'. (undefined)
(StaticAccess)
tests/Embedded/IframeTest.php
[warning] 39-1077: The class IframeTest has 1039 lines of code. Current threshold is 1000. Avoid really long classes. (undefined)
(ExcessiveClassLength)
[warning] 39-1077: The class IframeTest has 70 public methods and attributes. Consider reducing the number of public items to less than 45. (undefined)
(ExcessivePublicCount)
[warning] 39-1077: The class IframeTest has 70 non-getter- and setter-methods. Consider refactoring IframeTest to keep number of methods under 25. (undefined)
(TooManyMethods)
[warning] 39-1077: The class IframeTest has 70 public methods. Consider refactoring IframeTest to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
[warning] 39-1077: The class IframeTest has an overall complexity of 70 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
[error] 39-1077: The class IframeTest has a coupling between objects value of 22. Consider to reduce the number of dependencies under 13. (undefined)
(CouplingBetweenObjects)
[error] 254-254: Avoid using static access to class '\UIAwesome\Html\Embedded\Iframe' in method 'testRenderWithBeginEnd'. (undefined)
(StaticAccess)
[error] 440-443: Avoid using static access to class '\UIAwesome\Html\Core\Factory\SimpleFactory' in method 'testRenderWithGlobalDefaultsAreApplied'. (undefined)
(StaticAccess)
[error] 454-457: Avoid using static access to class '\UIAwesome\Html\Core\Factory\SimpleFactory' in method 'testRenderWithGlobalDefaultsAreApplied'. (undefined)
(StaticAccess)
[error] 838-838: Avoid using static access to class '\UIAwesome\Html\Embedded\Iframe' in method 'testRenderWithToString'. (undefined)
(StaticAccess)
[error] 873-879: Avoid using static access to class '\UIAwesome\Html\Core\Factory\SimpleFactory' in method 'testRenderWithUserOverridesGlobalDefaults'. (undefined)
(StaticAccess)
[error] 890-893: Avoid using static access to class '\UIAwesome\Html\Core\Factory\SimpleFactory' in method 'testRenderWithUserOverridesGlobalDefaults'. (undefined)
(StaticAccess)
[error] 898-898: Avoid using static access to class '\UIAwesome\Html\Embedded\Iframe' in method 'testReturnNewInstanceWhenSettingAttribute'. (undefined)
(StaticAccess)
[error] 959-959: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingContentEditable'. (undefined)
(StaticAccess)
[error] 959-959: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\ContentEditable' in method 'testThrowInvalidArgumentExceptionWhenSettingContentEditable'. (undefined)
(StaticAccess)
[error] 973-973: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingDir'. (undefined)
(StaticAccess)
[error] 973-973: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Direction' in method 'testThrowInvalidArgumentExceptionWhenSettingDir'. (undefined)
(StaticAccess)
[error] 987-987: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingDraggable'. (undefined)
(StaticAccess)
[error] 987-987: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Draggable' in method 'testThrowInvalidArgumentExceptionWhenSettingDraggable'. (undefined)
(StaticAccess)
[error] 1001-1001: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingLang'. (undefined)
(StaticAccess)
[error] 1001-1001: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Language' in method 'testThrowInvalidArgumentExceptionWhenSettingLang'. (undefined)
(StaticAccess)
[error] 1015-1015: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingLoading'. (undefined)
(StaticAccess)
[error] 1015-1015: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Loading' in method 'testThrowInvalidArgumentExceptionWhenSettingLoading'. (undefined)
(StaticAccess)
[error] 1029-1029: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingReferrerpolicy'. (undefined)
(StaticAccess)
[error] 1029-1029: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Referrerpolicy' in method 'testThrowInvalidArgumentExceptionWhenSettingReferrerpolicy'. (undefined)
(StaticAccess)
[error] 1043-1043: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingRole'. (undefined)
(StaticAccess)
[error] 1043-1043: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Role' in method 'testThrowInvalidArgumentExceptionWhenSettingRole'. (undefined)
(StaticAccess)
[error] 1071-1071: Avoid using static access to class '\UIAwesome\Html\Helper\Enum' in method 'testThrowInvalidArgumentExceptionWhenSettingTranslate'. (undefined)
(StaticAccess)
[error] 1071-1071: Avoid using static access to class '\UIAwesome\Html\Attribute\Values\Translate' in method 'testThrowInvalidArgumentExceptionWhenSettingTranslate'. (undefined)
(StaticAccess)
🔇 Additional comments (5)
CHANGELOG.md (1)
11-11: LGTM!README.md (1)
31-32: AI summary inconsistency: source element count and img change status.The AI summary describes swapping the order of two
<source>elements (mobile and desktop), but only one<source>element is present in the code. Additionally, the summary claims the<img>element "remains the same," yet line 32 is marked as modified.scaffold-lock.json (1)
8-8: AI summary error: wrong provider name.The AI summary states the
php-forge/baselineprovider was updated, but the actual change is to thephp-forge/coding-standardprovider version.src/Embedded/Iframe.php (1)
1-252: LGTM!tests/Embedded/IframeTest.php (1)
1-746: LGTM!Also applies to: 775-1077
1 participant
Pull Request