-
-
Notifications
You must be signed in to change notification settings - Fork 236
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
Add BitMarkdownEditor component (#10230) #10268
Add BitMarkdownEditor component (#10230) #10268
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a new MarkdownEditor component for the BlazorUI project. It adds the Razor component along with its C# code-behind, supporting SCSS and TypeScript files, and JavaScript runtime extension methods. The changes enable Markdown editing with JS interop, including initialization and value retrieval functionality. In addition, demo pages, navigation links, and compiler configuration updates have been added to showcase the component. A minor adjustment in BitComponentBase improves parameter initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as BitMarkdownEditor Component
participant JS as IJSRuntime / JS
U->>C: Load page (renders component)
C->>JS: Call BitMarkdownEditorInit(_Id, _textAreaRef)
JS-->>C: Initialization complete
U->>C: Enter markdown text & trigger GetValue()
C->>JS: Call BitMarkdownEditorGetValue(_Id)
JS-->>C: Return current markdown value
C-->>U: Display markdown value
Assessment against linked issues
Poem
🪧 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
🧹 Nitpick comments (14)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor.cs (3)
5-7
: Empty component parameters list.The componentParameters list is initialized as empty. Consider either adding parameters or removing the unused list if it's not needed for the current implementation.
9-11
: Unnecessary blank lines.Consider removing extra blank lines to maintain consistent code style.
18-20
: Unnecessary blank lines.Consider removing extra blank lines to maintain consistent code style.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor (2)
1-5
: Consider refining description text.“markdowneditor component of the bit BlazorUI components” can be rephrased for clarity and consistent branding (e.g., “MarkdownEditor component for Bit.BlazorUI”).
- Description="markdowneditor component of the bit BlazorUI components" + Description="MarkdownEditor component for Bit.BlazorUI"
7-21
: Avoid inline styles for better maintainability.Inline styles (e.g.,
style="margin-top:1rem;display:flex;..."
) may make future maintenance more cumbersome. Consider moving them to a dedicated SCSS file and referencing a class instead.src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.razor (2)
1-3
: Remove unexpected BOM character.Line 1 contains a Byte Order Mark (BOM) or a hidden character. This can cause issues in some editors and version control diffs. Consider removing it for consistency.
4-11
: Add accessibility attributes to the <textarea>.There is no label or ARIA attribute associated with the
<textarea>
. This might hinder screen readers. Provide either a visible label or aria-label for better accessibility.<textarea @ref="_textAreaRef" class="bit-mde-txa" + aria-label="Markdown Editor" />
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditorJsRuntimeExtensions.cs (1)
3-15
: Potential improvement: typed identifiers.This approach with a string-based
id
works but can make code error-prone ifid
is incorrectly spelled. Consider using a more strongly typed structure or a specialized class for referencing editor instances.src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.ts (2)
16-22
: Return consistent type from getValue().If
MarkdownEditor._editors[id]
is missing,undefined
is returned. Otherwise, a string. Consider returning a default string or throwing an error if no editor is found, to avoid ambiguities.public static getValue(id: string) { const editor = MarkdownEditor._editors[id]; if (!editor) { - return; + return ""; // or throw new Error(`No editor found for ID: ${id}`); } return editor.value; }
201-333
: Consider removing event listeners on teardown to prevent memory leaks.The added event listeners remain active if this element is removed from the DOM. Implement a cleanup routine (e.g., through a dispose pattern) to detach them.
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.razor.cs (4)
8-12
: Consider adding configuration options and parameters.The component currently lacks customization parameters that might be useful for a markdown editor, such as:
- Initial value/content
- Placeholder text
- Read-only mode
- Custom toolbar options
- Height/width settings
Consider adding these as public parameters to enhance the component's flexibility.
private ElementReference _textAreaRef = default!; +/// <summary> +/// Gets or sets the initial value of the markdown editor. +/// </summary> +[Parameter] public string? Value { get; set; } + +/// <summary> +/// Gets or sets the placeholder text for the editor. +/// </summary> +[Parameter] public string? Placeholder { get; set; } + +/// <summary> +/// Gets or sets whether the editor is in read-only mode. +/// </summary> +[Parameter] public bool ReadOnly { get; set; } = false; [Inject] private IJSRuntime _js { get; set; } = default!;
16-19
: Add error handling to the JavaScript interop call.JavaScript interop calls can fail for various reasons. Consider adding try-catch blocks to handle potential errors gracefully.
public async ValueTask<string> GetValue() { - return await _js.BitMarkdownEditorGetValue(_Id); + try + { + return await _js.BitMarkdownEditorGetValue(_Id); + } + catch (Exception ex) + { + Console.Error.WriteLine($"Error getting markdown editor value: {ex.Message}"); + return string.Empty; + } }
25-33
: Add error handling and pass configuration parameters to initialization.The initialization code should handle potential JavaScript errors and pass any configuration parameters to the JavaScript initialization function.
protected override Task OnAfterRenderAsync(bool firstRender) { if (firstRender) { - _js.BitMarkdownEditorInit(_Id, _textAreaRef); + try + { + _js.BitMarkdownEditorInit(_Id, _textAreaRef, new + { + value = Value, + placeholder = Placeholder, + readOnly = ReadOnly + }); + } + catch (Exception ex) + { + Console.Error.WriteLine($"Error initializing markdown editor: {ex.Message}"); + } } return base.OnAfterRenderAsync(firstRender); }
1-34
: Consider adding event callbacks and additional methods for a complete component.The current implementation is missing some features that would make it more versatile:
- Event callbacks for value changes
- Method to set the value programmatically
- Disposal pattern for proper cleanup
Here are suggested additions to the component:
/// <summary> /// Event callback that fires when the editor's value changes. /// </summary> [Parameter] public EventCallback<string> OnValueChange { get; set; } /// <summary> /// Sets the value of the markdown editor. /// </summary> /// <param name="value">The new value to set.</param> public async Task SetValue(string value) { try { await _js.BitMarkdownEditorSetValue(_Id, value); } catch (Exception ex) { Console.Error.WriteLine($"Error setting markdown editor value: {ex.Message}"); } } public async ValueTask DisposeAsync() { try { await _js.BitMarkdownEditorDispose(_Id); } catch (Exception ex) { Console.Error.WriteLine($"Error disposing markdown editor: {ex.Message}"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.razor
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.razor.cs
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.scss
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.ts
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditorJsRuntimeExtensions.cs
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss
(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/BitComponentBase.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor.scss
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Home/ComponentsSection.razor
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.ts
[error] 176-176: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (14)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.scss (1)
1-12
: Clean and consistent styling approach.The SCSS file provides simple and effective styling for the markdown editor component. The styles follow the component's naming convention with the "bit-" prefix and use the imported spacing function for consistent padding.
src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss (1)
6-6
: Import correctly added.The import statement for the BitMarkdownEditor styles has been properly added, maintaining the alphabetical ordering of the imports.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor.scss (1)
1-7
: Responsive image styling properly implemented.The SCSS file correctly uses the
::deep
selector to style images within elements with the.advanced
class, ensuring they don't overflow their containers. This follows responsive design best practices.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs (1)
158-158
: Navigation item added correctly.The new navigation item for the MarkdownEditor component follows the established pattern and is appropriately placed in the "Extras" category with proper URL and description.
src/BlazorUI/Bit.BlazorUI/Components/BitComponentBase.cs (1)
81-81
: Good robust solution for parameter handling.This change improves error resilience by initializing a new dictionary from parameters when ParametersCache is null, instead of potentially throwing an exception. This makes components more robust and better handles edge cases where ParametersCache might not be initialized.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json (1)
80-85
: Consistent configuration for the new MarkdownEditor component.The configuration follows the same pattern as other components with minification and source maps disabled, maintaining consistency across the project.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Home/ComponentsSection.razor (1)
262-264
: Well-placed navigation link for the new MarkdownEditor component.The link maintains alphabetical ordering in the Extras section and follows the consistent style pattern used for other components.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MarkdownEditor/BitMarkdownEditorDemo.razor.cs (2)
11-16
: Well-implemented editor reference and value retrieval.The implementation correctly handles the reference to the BitMarkdownEditor component and provides an asynchronous method to retrieve its value.
20-22
: Good sample code inclusion for documentation.Including example code for the related BitMarkdownViewer component provides useful context for users.
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditorJsRuntimeExtensions.cs (1)
1-2
: Check for unexpected characters or spacing.Ensure no stray punctuation or BOM-like characters are left in the namespace line.
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.ts (2)
172-178
: Ignore static analysis recommendation about the 'continue' statement.The static analysis flags this as unnecessary, but it’s actually integral to skip the subsequent else block and proceed to the next iteration. Removing it would alter logic.
🧰 Tools
🪛 Biome (1.9.4)
[error] 176-176: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
92-117
: Watch out for performance overhead from frequent string slicing.Repeated string slicing (lines 98-105) can become expensive for large text content. Consider buffered or more efficient manipulations if performance becomes a concern.
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownEditor/BitMarkdownEditor.razor.cs (2)
3-7
: LGTM: Clean class declaration and appropriate summary.The class summary clearly describes the purpose of the component. The partial class design allows proper separation of concerns between the markup (Razor) and code-behind.
23-23
: LGTM: Proper override of RootElementClass.The CSS class naming follows the component's naming convention.
closes #10230
Summary by CodeRabbit