-
-
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
Improve sales module ui in Boilerplate (#9807) #9809
Improve sales module ui in Boilerplate (#9807) #9809
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 updates the Bit Boilerplate template. The changes include reformatting the template configuration to improve readability and expanding the exclusion list for module-specific files. In the UI layer, the IdentityHeader component’s theme-based icon logic is reversed, and MainLayout now uses an AutoGoToTop attribute. The HomePage has been refactored to remove inline Sales product logic and instead rely on new modular components (ProductsCarousel and ProductsSection) with updated code-behind and SCSS imports. Additionally, the default product image has been updated from PNG to SVG, and the ProductViewController now returns six items rather than ten. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProductsCarousel
participant ProductViewController
User->>ProductsCarousel: Load Carousel
ProductsCarousel->>ProductViewController: GetHomeCarouselProducts(cancellationToken)
ProductViewController-->>ProductsCarousel: Return 6 products
ProductsCarousel-->>User: Render Carousel with Products
sequenceDiagram
participant User
participant ProductsSection
participant ProductViewController
User->>ProductsSection: Scroll / Request More Products
ProductsSection->>ProductViewController: LoadProducts(request)
ProductViewController-->>ProductsSection: Return paged product data
ProductsSection-->>User: Render additional products
Assessment against linked issues
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: 2
🧹 Nitpick comments (7)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsCarousel.razor.cs (1)
6-8
: Add XML documentation for the component.Consider adding XML documentation to describe the component's purpose and usage.
+/// <summary> +/// Displays products in a carousel format on the home page. +/// </summary> public partial class ProductsCarousel {src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsSection.razor.cs (1)
16-16
: Consider parameterizing the page size.The hardcoded value of 10 for
Top
should be configurable to allow for different page sizes.+ private const int DefaultPageSize = 10; + + [Parameter] public int PageSize { get; set; } = DefaultPageSize; + private async ValueTask<IEnumerable<ProductDto>> LoadProducts(BitInfiniteScrollingItemsProviderRequest request) { try { return await productViewController - .WithQueryString(new ODataQuery { Top = 10, Skip = request.Skip }) + .WithQueryString(new ODataQuery { Top = PageSize, Skip = request.Skip })src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsSection.razor (1)
5-14
: Enhance accessibility for product items.Add ARIA labels and roles to improve accessibility for screen readers.
- <BitLink Href="@($"/product/{product.Id}")" NoUnderline Class="product-item"> + <BitLink Href="@($"/product/{product.Id}")" NoUnderline Class="product-item" + aria-label="@($"View details for {product.Name}")"> <BitCard FullSize> - <BitStack Class="product-stack"> + <BitStack Class="product-stack" role="article"> <ProductImage Src="@GetProductImageUrl(product)" Width="100%" /> <BitText>@product.Name</BitText> <BitText Typography="BitTypography.Body2">@product.Description</BitText> <BitText Typography="BitTypography.H6">@product.FormattedPrice</BitText> </BitStack> </BitCard> </BitLink>src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductViewController.cs (2)
33-44
: Improve similar products implementation.The current implementation using random ordering might not provide the most relevant similar products. Consider implementing a more sophisticated similarity algorithm based on product attributes.
Would you like me to propose a more sophisticated implementation for finding similar products based on category, price range, and other attributes?
19-19
: Document the reason for limiting carousel products.Add a comment explaining why the number of carousel products is limited to 6.
+ // Limiting to 6 products for optimal carousel display on various screen sizes return await Get().Take(6).ToListAsync(cancellationToken);
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsCarousel.razor (2)
34-34
: Add descriptive text to the Buy button.The Buy button lacks descriptive text for screen readers. Consider adding an ARIA label with the product name.
- <BitButton Color="BitColor.Tertiary">Buy</BitButton> + <BitButton Color="BitColor.Tertiary" AriaLabel="@($"Buy {product.Name}")">Buy</BitButton>
24-24
: Consider adding pause controls for the carousel autoplay.While autoplay can enhance engagement, it may cause accessibility issues. Consider adding controls to pause the autoplay.
- <BitCarousel @ref="carouselRef" Class="carousel" AutoPlay AutoPlayInterval="5000" HideNextPrev InfiniteScrolling> + <BitCarousel @ref="carouselRef" Class="carousel" AutoPlay AutoPlayInterval="5000" HideNextPrev InfiniteScrolling AllowPause>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/wwwroot/images/product-placeholder.png
is excluded by!**/*.png
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/wwwroot/images/product-placeholder.svg
is excluded by!**/*.svg
📒 Files selected for processing (13)
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json
(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor.scss
(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsCarousel.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsCarousel.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsSection.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/ProductsSection.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/ProductImage.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/compilerconfig.json
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductViewController.cs
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/ProductImage.razor
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (9)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
10-10
: LGTM! Good UX improvement.Adding
AutoGoToTop
improves user experience by automatically scrolling to the top when navigating between pages, which is particularly beneficial for the sales module with long product listings.Please verify that this behavior:
- Works correctly with anchor links (e.g.,
#section-id
)- Preserves scroll position when using browser back/forward navigation
src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json (2)
363-367
: Enhanced formatting for postActions file arguments.
This update improves readability by formatting the "files" array with line breaks under the "args" property for the restore project dependencies action. Please verify that"Boilerplate.Web.slnf"
is the correct file intended for restoration.
444-447
: Updated exclusion list for Sales module UI components and assets.
The new exclusions now cover theProductsCarousel
,ProductsSection
, and the updated product page component patterns, as well as the SVG placeholder image. Confirm that these patterns accurately match all files that should be excluded for non-Sales module builds according to the updated UI structure.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/compilerconfig.json (1)
76-77
: Updated HomePage Asset Paths
The output and input file paths for the HomePage component have been moved into a dedicated Home folder (Components/Pages/Home/
). This revision enhances the project’s organization and aligns with the recent refactor where HomePage logic has been modularized. Please ensure that every reference to these style files in the codebase (e.g., in component imports or build configurations) is updated accordingly.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor.cs (1)
5-5
: LGTM! Good namespace organization.The namespace change to
Boilerplate.Client.Core.Components.Pages.Home
better reflects the component's location and purpose.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor (1)
23-27
: LGTM! Clean component integration.The integration of ProductsCarousel and ProductsSection components improves code organization and maintainability by separating concerns.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Home/HomePage.razor.scss (3)
2-2
: Verify Correctness of Updated Import Path
The media queries import path has been updated to'../../../Styles/abstracts/_media-queries.scss'
. Please double-check that this new relative path correctly resolves to the intended file given the updated directory structure.
37-45
: Review Newly Introduced.carousel-buttons
Styles
The new.carousel-buttons
class is added with absolute positioning (usingbottom: 1rem
andinset-inline-end: 1rem
) and hides the buttons on small screens via thelt-sm
media query. Verify that these styles meet the design and responsiveness requirements for the carousel functionality on all targeted devices.
50-50
: Confirm Padding Update in.products-inf-scr
The padding for the.products-inf-scr
class has been updated to3px
. Ensure that this spacing change maintains overall layout consistency and aligns with the design specifications for the HomePage component.
closes #9807
Summary by CodeRabbit
New Features
Refactor
Style
Chores