LumexImage#273
Conversation
Introduce LumexImage, a customizable Blazor image component supporting object fit, blur, radius, shadow, and more. Replace all <img> tags in demo components with <LumexImage> for consistent styling. Add supporting enums, style logic, and a basic render test. Update solution file accordingly.
Introduced a GrayScale property to LumexImage, enabling grayscale rendering via a CSS class. Updated the Image class to apply the "grayscale" class when GrayScale is true. Also added pointer and hover styles when an OnClick handler is present. All changes are integrated into the GetStyles method for consistent styling.
Created /docs/components/image with interactive examples for all LumexImage features, including sizing, fit, radius, shadow, blur, grayscale, click handling, and custom styles. Updated navigation to include LumexImage and marked it as "New" where appropriate. All examples use PreviewCode and InteractiveWebAssembly rendering.
📝 WalkthroughWalkthroughAdds a new LumexImage component with supporting enums, styles, tests, and documentation examples; replaces raw img tags in docs with LumexImage; updates docs navigation and a solution metadata change. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@docs/LumexUI.Docs.Client/Pages/Components/Image/Examples/ImageRadius.razor`:
- Around line 1-7: Add descriptive Alt attributes to each LumexImage example so
the sample demonstrates accessible usage; update the five LumexImage instances
(the ones using Radius.None, Radius.Small, Radius.Medium, Radius.Large,
Radius.Full) to include Alt strings like "Radius none example", "Radius small
example", etc., ensuring each image has a meaningful alt attribute.
In `@docs/LumexUI.Docs.Client/Pages/Components/Image/Image.razor`:
- Around line 89-106: The "Custom Styles" docs incorrectly mention the Classes
dictionary for targeting internal slots even though the LumexImage component
renders only a single <img> element; update the Image.razor docs to remove or
change the Classes bullet so it no longer suggests internal slot
targeting—either delete the `<code>Classes</code>` list item or replace it with
a clarifying sentence stating that LumexImage only supports `Class` and `Style`
(no internal slots), referencing the component name LumexImage and the `Class`
and `Style` parameters to guide the change.
- Around line 72-75: The DocsSection title uses "GrayScale" but the in-text
<code> shows "Grayscale"; update the documentation so the <code> tag matches the
actual component parameter name GrayScale (as defined in LumexImage.razor.cs) to
ensure consistent casing—locate the DocsSection Title="GrayScale" block and
change the inline <code> content from "Grayscale" to "GrayScale".
In `@src/LumexUI/Components/Image/LumexImage.razor`:
- Line 5: Fix the typo in the inline comment in LumexImage.razor: change "bigger
den zero" to "bigger than zero" in the comment that explains adding width and
height only when values are greater than zero (the comment above the
width/height handling logic in the LumexImage.razor component).
In `@src/LumexUI/Components/Image/LumexImage.razor.cs`:
- Around line 15-17: Update the XML documentation in LumexImage.razor.cs to
replace incorrect "button" references with "image": change the component summary
to "A component representing an image.", update the property/event docs (e.g.,
the "radius of the button" summary to "radius of the image" and "button is
clicked" to "image is clicked"), and correct any stray type/name mentions from
LumexButton to LumexImage (inspect the LumexImage class, its Radius property and
click event XML comments and update them accordingly).
In `@src/LumexUI/Styles/Image.cs`:
- Around line 45-52: Rename the misleading parameter in GetBlurStyles from
shadow to blur and update the method signature and all internal references (the
parameter checks like "when: shadow is Blur.None" -> "when: blur is Blur.None");
also update any callers of GetBlurStyles to pass a variable named blur (or keep
callers unchanged if they pass by position) so the symbol name matches intent
and compiles cleanly; ensure the method signature and any XML docs or tests
referencing GetBlurStyles are updated to the new parameter name.
- Around line 54-62: GetObjectFitStyles currently adds "object-none" when
LumexImage.Fit is the enum default (ObjectFit.None), causing unwanted
object-none being applied; update the caller in GetStyles to only invoke
GetObjectFitStyles when LumexImage.Fit has been explicitly set (i.e., Fit !=
default(ObjectFit) or Fit != ObjectFit.None), so default/unspecified images
don't receive the object-none class; keep GetObjectFitStyles unchanged but make
its usage conditional based on LumexImage.Fit.
In `@tests/LumexUI.Tests/Components/Image/ImageTests.cs`:
- Around line 17-23: The test method is misnamed: change the method name
Button_ShouldRenderCorrectly to Image_ShouldRenderCorrectly so it accurately
reflects the component under test; locate the test method that calls
RenderComponent<LumexImage>() (currently named Button_ShouldRenderCorrectly) and
rename the method to Image_ShouldRenderCorrectly, keeping the body and assertion
(action.Should().NotThrow()) unchanged.
🧹 Nitpick comments (4)
docs/LumexUI.Docs.Client/Pages/Components/Image/Examples/CustomStyles.razor (1)
1-3: Consider adding anAltattribute for accessibility.This example (and several other new Image examples like
ImageGrayscale,ImageWidthHeight) omit theAltprop. Since these serve as documentation examples that users will copy, it would be good practice to demonstrate accessible image usage with descriptive alt text.Proposed fix
<LumexImage Src="https://picsum.photos/id/101/400/300" - Class="border-4 border-blue-500 rounded-xl shadow-xl" /> + Alt="Sample image" + Class="border-4 border-blue-500 rounded-xl shadow-xl" />docs/LumexUI.Docs.Client/Pages/Components/Image/Examples/ImageWidthHeight.razor (1)
1-4: Grid declares 3 columns but only contains 2 items.
grid-cols-3with only 2 children leaves an empty cell. Consider usinggrid-cols-2unless the extra space is intentional for visual layout purposes.tests/LumexUI.Tests/Components/Image/ImageTests.cs (1)
17-23: Test coverage is very thin — consider adding tests for key parameters and theOnClickcallback.The component exposes
Blur,ObjectFit,Radius,Shadow,GrayScale,Width/Height,FullWidth/FullHeight, andOnClick. A single "doesn't throw" test leaves these untested. At minimum, verifying that setting parameters produces expected CSS classes (viaRootClass/TwMerge) and thatOnClickfires would strengthen confidence.src/LumexUI/Components/Image/LumexImage.razor.cs (1)
23-28:SrcandAltare non-nullable strings with no default value.In Blazor, unset parameters will be
nullat runtime despite the non-nullable declaration, which could cause NullReferenceException or compiler warnings. Consider defaulting tostring.Emptyor making them nullable.Proposed fix
- [Parameter] public string Src { get; set; } + [Parameter] public string Src { get; set; } = string.Empty; - [Parameter] public string Alt { get; set; } + [Parameter] public string Alt { get; set; } = string.Empty;
- Add Alt="Lumex Image" to all LumexImage usages in demo/example .razor files for accessibility and HTML validity. - Update Image.razor docs: fix grayscale parameter typo and clarify custom styles usage. - Correct XML docs in LumexImage.razor.cs to refer to "image" instead of "button". - Fix parameter naming in GetBlurStyles (Image.cs) from shadow to blur. - Rename test method to Image_ShouldRenderCorrectly in ImageTests.cs. - Fix minor comment typo in LumexImage.razor.
|
Hey @kevinvenclovas, Thank you very much for the PR! I am sorry for the long wait. I will review this asap. |
desmondinho
left a comment
There was a problem hiding this comment.
Hey @kevinvenclovas,
Thank you very much for this PR -- it’s great work.
I noticed that the two components you implemented don't fully align with HeroUI's capabilities, structure, and/or styles. Since I'm aiming to achieve 100% parity, could you please make sure they closely follow the HeroUI components?
For reference: https://www.heroui.com/docs/components/image
| @namespace LumexUI | ||
| @inherits LumexComponentBase | ||
|
|
||
| @{ |
There was a problem hiding this comment.
Let's use width and height attributes directly.
| [Parameter] public string Src { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the alt of the image. | ||
| /// </summary> | ||
| [Parameter] public string Alt { get; set; } |
There was a problem hiding this comment.
Missing nullability operators -- compiler's gonna warn
There was a problem hiding this comment.
No need for this file since it's an abstraction for a simple CSS classes. We don't do that. The only exception is Radius, though I wish I didn't introduce it :D
Description
Hi, want to help out on this nice repo and started with a small image component. Please if there is something to change let me know :)
What's been done?
Checklist
Additional Notes
Summary by CodeRabbit
New Features
Documentation
Tests
Chore