-
-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show missing required asterisk in BitTimePicker's label (#9893) #9894
Show missing required asterisk in BitTimePicker's label (#9893) #9894
Conversation
WalkthroughThis update introduces a required field feature for the TimePicker component. The modifications add a new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as BitTimePicker Component
participant I as Input Element
participant S as Stylesheet
U->>T: Access TimePicker demo page
T->>I: Render input with required attribute if Required is true
T->>T: Check IsEnabled and Required properties
T->>S: Register CSS class "bit-tpc-req" conditionally
S-->>T: Apply asterisk styling to label
T-->>U: Display updated TimePicker with required indicator
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this 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
🔭 Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TimePicker/BitTimePickerDemo.razor.cs (1)
5-222
: 🛠️ Refactor suggestionAdd documentation for the Required parameter.
The Required parameter is demonstrated in the example but not documented in the componentParameters list. Add the parameter documentation to maintain consistency.
new() { Name = "Responsive", Type = "bool", DefaultValue = "false", Description = "Enables the responsive mode in small screens.", }, + new() + { + Name = "Required", + Type = "bool", + DefaultValue = "false", + Description = "Whether the TimePicker input is required.", + }, new() { Name = "ShowCloseButton", Type = "bool",
🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.scss (1)
378-384
: LGTM! Consider adding aria-required for better accessibility.The implementation follows the project's styling patterns and uses consistent spacing and color variables. The required field indicator is visually clear and well-positioned.
While the visual indicator is good, ensure that the corresponding HTML template includes
aria-required="true"
on the input element for better accessibility. This complements the visual asterisk with proper semantic meaning for screen readers.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TimePicker/BitTimePickerDemo.razor.cs (1)
592-592
: LGTM! Consider enhancing the example with a placeholder.The example effectively demonstrates the Required parameter. To make it more consistent with other examples and improve user experience, consider adding a placeholder text.
-<BitTimePicker Label="Required" Required /> +<BitTimePicker Label="Required" Required Placeholder="Select a time..." />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.razor
(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.razor.cs
(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.scss
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TimePicker/BitTimePickerDemo.razor
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TimePicker/BitTimePickerDemo.razor.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (4)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.razor (1)
36-36
: LGTM! The required attribute is properly implemented.The required attribute is correctly bound to the Required property, enabling HTML5 form validation when needed.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/_Pickers/TimePicker/BitTimePicker.razor.cs (1)
319-319
: LGTM! The required CSS class is properly registered.The CSS class registration correctly considers both IsEnabled and Required states, following the component's naming conventions.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/TimePicker/BitTimePickerDemo.razor (2)
21-21
: LGTM! The required TimePicker example is well placed.The example clearly demonstrates how to use the Required feature in a simple and understandable way.
592-592
: LGTM! The example code is properly updated.The example code accurately reflects the implementation and maintains consistent formatting.
This closes #9893
Summary by CodeRabbit