Add semantic HTML time element support to LocalTimeText component #7
Conversation
…rameter and adjust tests for expected markup output
…DisableTimeElement parameter and semantic HTML wrapping
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LocalTimeText Component
participant Browser
User->>LocalTimeText Component: Render with Value, Format, [DisableTimeElement]
alt DisableTimeElement is false (default)
LocalTimeText Component->>Browser: Output <time datetime="...">formatted value</time>
else DisableTimeElement is true
LocalTimeText Component->>Browser: Output formatted value (no <time>)
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
28-34: Improve parameter documentation for clarity.The documentation should explicitly state what happens for both
trueandfalsevalues to avoid ambiguity./// <summary> -/// Gets or sets whether to wrap the output in an HTML <time> element with a datetime attribute. -/// When false, generates a semantic <time> tag with ISO-8601 datetime attribute for accessibility. -/// The default value is false. +/// Gets or sets whether to disable wrapping the output in an HTML <time> element. +/// When false (default), generates a semantic <time> tag with ISO-8601 datetime attribute for accessibility. +/// When true, renders plain text output for backward compatibility. /// </summary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
example/BlazorLocalTimeSample/Pages/Home.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor.cs(2 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(1 hunks)tests/BlazorLocalTimeTest/Component/LocalTimeTotal1.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeTextTest.razor(4 hunks)
🔇 Additional comments (8)
src/BlazorLocalTime/Components/LocalTimeText.razor (1)
3-10: All properties implemented; changes approvedI’ve confirmed that both
FormattedValueandIsoDateTimeare properly defined insrc/BlazorLocalTime/Components/LocalTimeText.razor.cs. The conditional rendering now cleanly wraps the formatted time in a semantic<time>element by default, falling back to plain text when disabled—excellent for accessibility and backward compatibility.No further changes required.
tests/BlazorLocalTimeTest/Component/LocalTimeTotal1.razor (1)
5-5: LGTM! Proper test usage of the new parameter.The test correctly demonstrates usage of the
DisableTimeElementparameter, ensuring the component behavior is validated when the semantic HTML wrapper is disabled.tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
66-67: LGTM! Correctly structured public API addition.The new
DisableTimeElementparameter is properly defined with the appropriate component parameter attribute and boolean type. This represents a clean, non-breaking API extension.tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt (1)
66-67: LGTM! Consistent API across target frameworks.The
DisableTimeElementparameter definition is consistent between .NET 8 and .NET 9, ensuring proper cross-framework compatibility.example/BlazorLocalTimeSample/Pages/Home.razor (1)
56-64: Excellent documentation update with clear examples.The updated documentation effectively explains the new semantic HTML behavior, including:
- Clear explanation of the default
<time>element wrapping- Purpose and usage of the
DisableTimeElementparameter- Concrete example showing the expected HTML output with
datetimeattributeThis will help users understand both the accessibility benefits and practical usage.
tests/BlazorLocalTimeTest/LocalTimeTextTest.razor (2)
15-15: LGTM! Test assertions correctly updated for semantic HTML output.The updated assertions properly verify the new default behavior where the component wraps output in a semantic
<time>element with ISO-8601datetimeattribute.Also applies to: 30-30, 44-44, 57-57
60-72: Excellent test coverage for the new DisableTimeElement parameter.This test properly verifies that when
DisableTimeElementis true, the component renders plain text output without the semantic<time>wrapper, ensuring backward compatibility.src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
59-60: LGTM! ISO-8601 datetime formatting implemented correctly.The
IsoDateTimeproperty properly uses the round-trip format specifier ("O") to generate ISO-8601 formatted UTC datetime for the<time>element'sdatetimeattribute.
Summary
<time>element wrapper to LocalTimeText component with properdatetimeattribute for improved accessibility and semantic HTMLDisableTimeElementparameter to maintain backward compatibilityChanges
<time datetime="ISO-8601">formatted-time</time>by default<time>wrapper and get plain text (previous behavior)Test plan
<time>element with properdatetimeattribute by defaultDisableTimeElement=trueproduces plain text output (backward compatibility)DisableTimeElementparameterclose #6
Summary by CodeRabbit
<time>element wrapper.