Conversation
Add ThemeSecondaryColor, LogoUrl, FontFamily, BorderRadiusStyle, HeroBannerUrl, SuccessColor and ErrorColor to: - TenantInfoDto (Shared) - CreateTenantCommand / UpdateTenantCommand (Shared) - Tenant API model - Admin TenantEndpoints (create, update, list) - Public TenantInfoEndpoints (list, get)
…ont loader - TenantService: expose 7 new branding properties with sensible defaults - Routes.razor: build full MudTheme (PaletteLight + PaletteDark with all tokens, Typography font family, LayoutProperties border radius) - wwwroot/js/theme.js: dynamic Google Font loader via module upsert
- MainLayout: DrawerVariant.Mini (icon rail by default), collapsed on load, logo/image in AppBar when LogoUrl is set, borderless AppBar with backdrop-blur, cleaner AppBar layout - NavMenu: admin links grouped in collapsible MudNavGroup, tooltips on public links, better icons (MenuBook, Domain, LibraryBooks, LocalOffer) - MainLayout.razor.css: removed all dead .sidebar/.top-row/.page CSS - NavMenu.razor.css: removed all legacy .navbar-toggler/.bi-* SVG styles
…nding fields, CSS cleanup
…, and new grid layout
There was a problem hiding this comment.
Pull request overview
This pull request enhances the tenant branding system by adding seven new customization properties to the tenant model, expanding theme capabilities with full color palette and typography support, implementing a mini-drawer navigation pattern, and redesigning the home page with improved book card layouts and a more prominent hero section.
Changes:
- Extended tenant model with branding properties: secondary color, success/error colors, logo URL, hero banner URL, font family, and border radius style
- Enhanced theme builder in Routes.razor to support complete color palette, typography with Google Fonts, and configurable border radius
- Redesigned authentication pages (Login/Register) with split-panel layout featuring gradient backgrounds and tenant branding
- Improved home page with new hero banner, redesigned book cards with hover overlays, and generated gradient covers for books without images
- Implemented mini-drawer navigation defaulting to collapsed state with tooltips for accessibility
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BookStore.Web/wwwroot/js/theme.js | New JavaScript module for dynamically loading Google Fonts |
| src/BookStore.Web/wwwroot/app.css | Removed legacy Bootstrap styles, added auth page and utility classes |
| src/BookStore.Web/Services/TenantService.cs | Added 7 new tenant branding properties with defaults |
| src/BookStore.Web/Components/Routes.razor | Enhanced theme builder with typography, palette, and Google Font loading |
| src/BookStore.Web/Components/Pages/Register.razor | Redesigned with split-panel layout and tenant branding |
| src/BookStore.Web/Components/Pages/Login.razor | Redesigned with split-panel layout and tenant branding |
| src/BookStore.Web/Components/Pages/Home.razor.css | New scoped styles for book cards, covers, and filters |
| src/BookStore.Web/Components/Pages/Home.razor | Redesigned hero section, book cards with hover effects and gradient covers |
| src/BookStore.Web/Components/Pages/BookDetails.razor | Updated border radius values and improved add-to-cart button styling |
| src/BookStore.Web/Components/Pages/Admin/TenantManagement.razor | Extended form with new branding fields (colors, fonts, URLs, border radius) |
| src/BookStore.Web/Components/Layout/NavMenu.razor.css | Removed legacy styles, now fully MudBlazor-themed |
| src/BookStore.Web/Components/Layout/NavMenu.razor | Restructured with tooltips, grouped admin items, updated icons |
| src/BookStore.Web/Components/Layout/MainLayout.razor.css | Removed legacy layout styles |
| src/BookStore.Web/Components/Layout/MainLayout.razor | Updated AppBar with logo support, mini-drawer variant, changed default state |
| src/BookStore.Web/Components/Catalog/BookSaleBadge.razor | Updated badge design with icon and percentage formatting |
| src/BookStore.Shared/Models/TenantInfoDto.cs | Extended with 7 new optional branding parameters |
| src/BookStore.Shared/Models/TenantCommands.cs | Extended CreateTenant and UpdateTenant commands with new parameters |
| src/BookStore.ApiService/Models/Tenant.cs | Added 7 new nullable branding properties |
| src/BookStore.ApiService/Endpoints/TenantInfoEndpoints.cs | Updated to include new branding properties in responses |
| src/BookStore.ApiService/Endpoints/Admin/TenantEndpoints.cs | Updated CRUD operations to handle new branding properties |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <img src="@TenantService.CurrentTenantLogoUrl" alt="@TenantService.CurrentTenantName" | ||
| style="max-height: 80px; object-fit: contain;" | ||
| onerror="this.style.display='none'"/> |
There was a problem hiding this comment.
Same security issue - user-controlled URL (CurrentTenantLogoUrl) in img src without validation.
|
|
||
| @code { | ||
| bool _drawerOpen = true; | ||
| bool _drawerOpen = false; |
There was a problem hiding this comment.
The drawer defaults to closed (_drawerOpen = false) which may not be ideal for desktop users. Consider using a media query or checking screen size to keep it open by default on larger screens while closing it on mobile, providing a better default experience for desktop users.
|
|
||
| if (_jsAvailable) | ||
| { | ||
| _ = LoadGoogleFontAsync(); |
There was a problem hiding this comment.
The FireAndForget call to LoadGoogleFontAsync (line 137) discards the Task without awaiting or handling potential exceptions. While the method has internal exception handling, this pattern can make debugging difficult. Consider using a dedicated fire-and-forget helper method that logs exceptions, or use InvokeAsync to ensure proper exception handling in the Blazor component lifecycle.
| <MudContainer MaxWidth="MaxWidth.ExtraExtraLarge"> | ||
| @* ─── Hero Section ──────────────────────────────────────────────────── *@ | ||
| <MudPaper Elevation="0" Class="mb-6 hero-banner" | ||
| Style="@((!string.IsNullOrEmpty(TenantService.CurrentTenantHeroBannerUrl) |
There was a problem hiding this comment.
User-controlled URLs (LogoUrl and HeroBannerUrl) are directly interpolated into HTML attributes without validation or sanitization. This creates a potential XSS vulnerability if an admin enters a malicious URL like javascript:alert('XSS') or a data URI with embedded scripts. Consider adding URL validation to ensure only safe HTTP/HTTPS URLs are accepted, or sanitize the URLs before rendering. For example, you could validate that LogoUrl and HeroBannerUrl start with "http://" or "https://" in the TenantService or add validation rules to the Tenant model.
| Style="@((!string.IsNullOrEmpty(TenantService.CurrentTenantHeroBannerUrl) | |
| Style="@((!string.IsNullOrEmpty(TenantService.CurrentTenantHeroBannerUrl) | |
| && Uri.TryCreate(TenantService.CurrentTenantHeroBannerUrl, UriKind.Absolute, out var heroUri) | |
| && (heroUri.Scheme == Uri.UriSchemeHttp || heroUri.Scheme == Uri.UriSchemeHttps) |
| <img src="@TenantService.CurrentTenantLogoUrl" alt="@TenantService.CurrentTenantName" | ||
| style="height:34px; max-width:160px; object-fit:contain; margin-left:8px;" | ||
| onerror="this.style.display='none'"/> |
There was a problem hiding this comment.
User-controlled URL (CurrentTenantLogoUrl) used in img src without validation. Same XSS vulnerability as in Login and Register pages.
| !string.IsNullOrEmpty(info.ThemeSecondaryColor) ? info.ThemeSecondaryColor! : "#FF6B35"; | ||
| CurrentTenantLogoUrl = info.LogoUrl; | ||
| CurrentTenantFontFamily = | ||
| !string.IsNullOrEmpty(info.FontFamily) ? info.FontFamily! : "Inter"; | ||
| CurrentTenantBorderRadiusStyle = | ||
| !string.IsNullOrEmpty(info.BorderRadiusStyle) ? info.BorderRadiusStyle! : "rounded"; | ||
| CurrentTenantHeroBannerUrl = info.HeroBannerUrl; | ||
| CurrentTenantSuccessColor = | ||
| !string.IsNullOrEmpty(info.SuccessColor) ? info.SuccessColor! : "#2ECC71"; | ||
| CurrentTenantErrorColor = | ||
| !string.IsNullOrEmpty(info.ErrorColor) ? info.ErrorColor! : "#E74C3C"; |
There was a problem hiding this comment.
The null-forgiving operator (!) is used on properties that are already nullable (ThemeSecondaryColor, FontFamily, BorderRadiusStyle, SuccessColor, ErrorColor). This operator is unnecessary when the property is already nullable and you're checking for null/empty. The null-forgiving operator should only be used when you know the value is non-null but the compiler can't prove it.
| !string.IsNullOrEmpty(info.ThemeSecondaryColor) ? info.ThemeSecondaryColor! : "#FF6B35"; | |
| CurrentTenantLogoUrl = info.LogoUrl; | |
| CurrentTenantFontFamily = | |
| !string.IsNullOrEmpty(info.FontFamily) ? info.FontFamily! : "Inter"; | |
| CurrentTenantBorderRadiusStyle = | |
| !string.IsNullOrEmpty(info.BorderRadiusStyle) ? info.BorderRadiusStyle! : "rounded"; | |
| CurrentTenantHeroBannerUrl = info.HeroBannerUrl; | |
| CurrentTenantSuccessColor = | |
| !string.IsNullOrEmpty(info.SuccessColor) ? info.SuccessColor! : "#2ECC71"; | |
| CurrentTenantErrorColor = | |
| !string.IsNullOrEmpty(info.ErrorColor) ? info.ErrorColor! : "#E74C3C"; | |
| !string.IsNullOrEmpty(info.ThemeSecondaryColor) ? info.ThemeSecondaryColor : "#FF6B35"; | |
| CurrentTenantLogoUrl = info.LogoUrl; | |
| CurrentTenantFontFamily = | |
| !string.IsNullOrEmpty(info.FontFamily) ? info.FontFamily : "Inter"; | |
| CurrentTenantBorderRadiusStyle = | |
| !string.IsNullOrEmpty(info.BorderRadiusStyle) ? info.BorderRadiusStyle : "rounded"; | |
| CurrentTenantHeroBannerUrl = info.HeroBannerUrl; | |
| CurrentTenantSuccessColor = | |
| !string.IsNullOrEmpty(info.SuccessColor) ? info.SuccessColor : "#2ECC71"; | |
| CurrentTenantErrorColor = | |
| !string.IsNullOrEmpty(info.ErrorColor) ? info.ErrorColor : "#E74C3C"; |
| @@ -0,0 +1,25 @@ | |||
| /** | |||
| * Loads a Google Font dynamically by upserting a <link> tag in <head>. | |||
There was a problem hiding this comment.
The word "upserting" is non-standard technical jargon. Use "creating or updating" for clarity in documentation.
| * Loads a Google Font dynamically by upserting a <link> tag in <head>. | |
| * Loads a Google Font dynamically by creating or updating a <link> tag in <head>. |
| <MudGrid Spacing="0" Style="min-height: calc(100vh - 64px);"> | ||
| @* ─── Left Brand Panel ───────────────────────────────────────────── *@ | ||
| <MudItem xs="12" md="6" | ||
| Style="@($"background: linear-gradient(160deg, {TenantService.CurrentTenantPrimaryColor} 0%, {TenantService.CurrentTenantSecondaryColor} 100%); display: flex; align-items: center; justify-content: center; padding: 3rem;")"> |
There was a problem hiding this comment.
User-controlled color values (CurrentTenantPrimaryColor, CurrentTenantSecondaryColor) are directly interpolated into inline styles. If a malicious admin enters values like red;}</style><script>alert('XSS')</script><style>, this could lead to XSS attacks. While the MudColorPicker in TenantManagement.razor might provide some protection, color values should be validated server-side to ensure they match a safe pattern (e.g., hex colors like #RRGGBB or rgb(a) values). Consider adding validation to only accept valid CSS color formats.
| <MudGrid Spacing="0" Style="min-height: calc(100vh - 64px);"> | ||
| @* ─── Left Brand Panel ───────────────────────────────────────────── *@ | ||
| <MudItem xs="12" md="6" | ||
| Style="@($"background: linear-gradient(160deg, {TenantService.CurrentTenantPrimaryColor} 0%, {TenantService.CurrentTenantSecondaryColor} 100%); display: flex; align-items: center; justify-content: center; padding: 3rem;")"> |
There was a problem hiding this comment.
Same security issue as in Login.razor - user-controlled color values are interpolated into inline styles without validation. This could enable XSS if malicious color values are entered.
| <img src="@TenantService.CurrentTenantLogoUrl" alt="@TenantService.CurrentTenantName" | ||
| style="max-height: 80px; object-fit: contain;" | ||
| onerror="this.style.display='none'"/> |
There was a problem hiding this comment.
User-controlled URL (CurrentTenantLogoUrl) is used directly in img src attribute without validation. This could allow malicious URLs including javascript: or data: URIs that could execute scripts. Validate URLs server-side to only allow safe HTTP/HTTPS URLs.
Enhance the tenant model with new branding properties and expand the theme builder to support a full palette and typography. Introduce a mini-drawer navigation and redesign the home page hero, along with various UI improvements across the application.
Type of Change
Related Issues
Fixes #
Changes Made
Added seven new branding properties to the tenant model.
Expanded theme builder with a complete color palette and typography support.
Implemented mini-drawer navigation and improved AppBar layout.
Redesigned home page hero and polished BookSaleBadge.
Updated authentication pages and improved book card design.
Testing
Unit tests pass (
dotnet test)Build succeeds (
dotnet build)Code formatting is correct (
dotnet format --verify-no-changes)No new analyzer warnings
Manual testing completed
Checklist
My code follows the project's coding standards
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published
Screenshots (if applicable)
Additional Notes