Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions .squad/agents/legolas/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -649,3 +649,85 @@ Fix theme color selector persistence — selected color not surviving page reloa
### Outcome

✅ Theme color and brightness now persist correctly across page reloads and sessions. PR #239 opened, ready for review.

## Learnings

### 2025-07 — Button Variant Styling (Issue #292, branch squad/291-input-css-fine-tuning)

**Source of truth for app button styles:** `src/Web/Styles/input.css` under `@layer components`. This compiles to `src/Web/wwwroot/css/tailwind.css` (gitignored — regenerated by `npm run tw:build`).

**Tailwind v4 grouped selector pattern:** Use a shared multi-class selector to avoid duplicating base styles across variants:

```css
.btn-primary, .btn-secondary, .btn-warning, .btn-destructive {
@apply inline-flex items-center gap-2 ... focus-visible:ring-2 disabled:opacity-50;
}
```

Then each variant only declares its colour-specific overrides. This is idiomatic Tailwind v4 component authoring.

**Fixed colour palette — all four variants:** All button variants use fixed Tailwind palette classes, not `var(--primary-*)` theme tokens. `.btn-primary` is green, `.btn-secondary` is blue, `.btn-warning` is amber, and `.btn-destructive` is red. None shift when the user picks a different colour theme — the palette is intentionally static to give each variant a clear, invariant semantic meaning.

**Bootstrap-like interactive states checklist:**

- `cursor-pointer` + `select-none` — Bootstrap sets these on buttons
- `focus-visible:ring-2 focus-visible:ring-offset-1 focus-visible:ring-{color}` — replaces Bootstrap's box-shadow focus ring
- `disabled:opacity-50 disabled:cursor-not-allowed` — Bootstrap uses `opacity: 0.65`
- `active:scale-[0.98]` — subtle press affordance (Bootstrap uses active filter)

**ConfirmDeleteDialog.razor:** Was using hardcoded inline Tailwind for delete/cancel buttons (`bg-red-600 text-white ...`). Migrated to `.btn-destructive` / `.btn-secondary`. File: `src/Web/Features/BlogPosts/Delete/ConfirmDeleteDialog.razor`.

**ManageRoles.razor inline role chip buttons:** Left as-is — they are compact chip/badge-style elements (px-3 py-1 text-sm) with a different visual purpose. Not the same pattern as action buttons.

**Pre-existing branch changes:** The `squad/291-input-css-fine-tuning` branch had uncommitted Profile.razor changes that upgraded role badges from soft pastel (`bg-red-100 text-red-800`) to solid (`bg-red-700 text-white`). Three bUnit tests in `tests/Web.Tests.Bunit/Features/ProfileTests.cs` needed updating to match the current Profile.razor output.

**Key files:**

- Button CSS source: `src/Web/Styles/input.css`
- Confirm delete dialog: `src/Web/Features/BlogPosts/Delete/ConfirmDeleteDialog.razor`
- Profile badge tests: `tests/Web.Tests.Bunit/Features/ProfileTests.cs`
- Tailwind build: `npm run tw:build`

## Learnings

### 2026-05-07 — Issue #292 follow-up: btn-destructive consistency

**Task:** The inline delete button in `src/Web/Features/BlogPosts/List/Index.razor` had hardcoded Tailwind classes (`bg-red-600 text-white hover:bg-red-700 ...`) instead of the shared `btn-destructive` utility defined in `input.css`. The delete dialog already used `btn-destructive` (from the #292 main work), but the list page was inconsistent.

**Change:**

- Replaced `class="inline-block px-3 py-1 text-sm rounded font-medium bg-red-600 text-white hover:bg-red-700 transition"` with `class="btn-destructive"` on the delete button in `Index.razor`.
- Added bUnit test `BlogIndexUsesBtnDestructiveForInlineDeleteButton` to `RazorSmokeTests.cs` to lock the variant in place and prevent regression.

**Rule reinforced:** Any destructive action (delete) must always use `.btn-destructive`, never raw Tailwind. This keeps colour/dark-mode and spacing behaviour consistent across all delete surfaces.

**Test results:** Architecture.Tests 16/16, Web.Tests.Bunit 74/74 — all green.

## Learnings

### 2025-07 — PR #295 Review (dark-mode colours + PageHeadingComponent)

**What I reviewed:**

- `input.css` centralised button variants, fixed dark-mode heading/paragraph contrast, migrated body/table/form colours to primary palette
- New `PageHeadingComponent.razor` (shared heading + PageTitle wrapper)
- All feature pages (Create, Edit, Index, ManageRoles, Profile) adopt the new component
- `NavMenu.razor` switched from inline Tailwind utility string on `<nav>` to `class="nav"`
- `_Imports.razor` adds `@using MyBlog.Web.Components.Shared` for Features
- bUnit tests updated to match new colour classes

**Key findings (for future work):**

1. **`TextColorClass` is dead code** — All call sites pass `text-primary-900 dark:text-primary-50`. The `h1/h2/h3` base rules in `input.css` already apply exactly those colours. The parameter never overrides anything meaningful. Future components should omit it or give it a genuinely different default.

2. **`<header>` in `PageHeadingComponent` degrades accessibility** — Every page gets a second `<header>` landmark. Screen readers present all landmarks in a navigation shortcut list; multiple anonymous `<header>` elements create noise. Should use a plain `<div>` or `<section>` instead.

3. **`class="nav"` in NavMenu is misleading** — CSS uses `nav {}` element selector, not `.nav` class rule. The `class="nav"` attribute is redundant/dead code. Either rename the CSS to `.nav {}` or remove the class attribute from the element.

4. **`Profile.razor` still has explicit `@using MyBlog.Web.Components.Shared`** — Redundant after `_Imports.razor` was updated. Small but noisy.

5. **`btn-primary` is always green** — Decoupled from the theme's primary palette. May be intentional (semantic: success/action = green) but breaks the expectation that primary buttons track the selected colour theme.

6. **`form-input`/`form-label` bump to `text-lg font-semibold`** — Unusually large/bold for form field text; worth a visual QA check.

**Verdict:** Approved with concerns (items 1, 2, 3 are the meaningful ones for follow-up).
23 changes: 23 additions & 0 deletions .squad/decisions/inbox/legolas-pr295-review-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# PR #295 Review Fix Decisions

**Agent:** Legolas
**Date:** 2026-05-07
**Branch:** squad/291-input-css-fine-tuning

## Decisions Made

### `.container-card` gains `mx-auto px-4`

The `.container-card` utility was bare `max-w-7xl` with no centering or padding. Aligned it to match the app's shared layout pattern (consistent with how `nav` wraps `mx-auto max-w-7xl px-4`). Any future page wrapper that adopts `.container-card` will automatically center and pad correctly.

### `.btn-secondary` is a solid blue, not an outline button

The comment said "outline style" but the implementation was always solid blue fill. Chose to fix the comment (not the style) to preserve existing UX. The solid blue secondary button is the canonical pattern going forward.

### `PageHeadingComponent` falls back to `<h1>` on unknown `Level`

Added a `default` switch arm that renders `<h1>`. Predictable output over silent empty rendering.

### All four button variants use fixed palettes (no theme tokens)

All of `.btn-primary` (green), `.btn-secondary` (blue), `.btn-warning` (amber), `.btn-destructive` (red) use fixed Tailwind colour classes. None follow the user's colour-theme switch. The history.md learning entry from Issue #292 incorrectly stated that primary/secondary used `var(--primary-*)` tokens — corrected.
7 changes: 4 additions & 3 deletions src/Web/Components/Layout/MainLayout.razor
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
@inherits LayoutComponentBase


<div class="min-h-screen flex flex-col bg-primary-100 dark:bg-primary-800 text-primary-900 dark:text-primary-100">
<NavMenu/>
<div class="min-h-screen flex flex-col bg-primary-300 dark:bg-primary-800 text-primary-900 dark:text-primary-100">
<NavMenu />

<main id="main-content" tabindex="-1" class="mx-auto w-full max-w-7xl flex-1 px-4 pt-20 pb-8 outline-none focus:outline-none focus-visible:outline-none">
<main id="main-content" tabindex="-1"
class="mx-auto w-full max-w-7xl flex-1 px-4 pt-20 pb-8 outline-none focus:outline-none focus-visible:outline-none">
@Body
</main>

Expand Down
4 changes: 1 addition & 3 deletions src/Web/Components/Layout/NavMenu.razor
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<header>
<nav class="fixed top-0 left-0 right-0 z-50
bg-primary-500 dark:bg-primary-900
border-b-2 border-primary-300 dark:border-primary-700" aria-label="Main navigation">
<nav aria-label="Main navigation">
<div class="mx-auto max-w-7xl px-4">
<div class="flex items-center justify-between h-16">

Expand Down
48 changes: 48 additions & 0 deletions src/Web/Components/Shared/PageHeadingComponent.razor
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<PageTitle>@HeaderText</PageTitle>

<header class="container-card">

@switch (Level)
{
case "1":

<h1 class=@($"{TextColorClass}")>@HeaderText</h1>
Comment thread
mpaulosky marked this conversation as resolved.

break;

case "2":

<h2 class=@($"{TextColorClass}")>@HeaderText</h2>

break;

case "3":

<h3 class=@($"{TextColorClass}")>@HeaderText</h3>

break;

case "4":

<h4 class=@($"{TextColorClass}")>@HeaderText</h4>

break;

default:

<h1 class=@($"{TextColorClass}")>@HeaderText</h1>

break;
}

</header>

@code {

[Parameter] public string HeaderText { get; set; } = "My Blog";

[Parameter] public string Level { get; set; } = "1";

[Parameter] public string TextColorClass { get; set; } = string.Empty;

}
91 changes: 44 additions & 47 deletions src/Web/Features/BlogPosts/Create/Create.razor
Original file line number Diff line number Diff line change
@@ -1,69 +1,66 @@
@page "/blog/create"
@using MyBlog.Web.Features.BlogPosts.Create
@inject ISender Sender
@inject NavigationManager Navigation
@rendermode InteractiveServer
@attribute [Authorize(Roles = "Author,Admin")]

<PageTitle>Create Post</PageTitle>

<h1 class="text-3xl font-bold mb-6">Create Post</h1>
<PageHeadingComponent HeaderText="Create Post" Level="1" TextColorClass="text-primary-900 dark:text-primary-50" />

@if (_error is not null)
{
<div class="alert-error" role="alert">
<span>@_error</span>
<button type="button" class="alert-dismiss" @onclick="() => _error = null">&times;</button>
</div>
<div class="alert-error" role="alert">
<span>@_error</span>
<button type="button" class="alert-dismiss" @onclick="() => _error = null">&times;</button>
</div>
}

<div class="form-panel">
<EditForm Model="_model" OnValidSubmit="HandleSubmit">
<DataAnnotationsValidator />
<ValidationSummary class="mb-4" />
<EditForm Model="_model" OnValidSubmit="HandleSubmit">
<DataAnnotationsValidator />
<ValidationSummary class="mb-4" />

<div class="form-group">
<label class="form-label">Title</label>
<InputText class="form-input" @bind-Value="_model.Title" />
</div>
<div class="form-group">
<label class="form-label">Author</label>
<InputText class="form-input" @bind-Value="_model.Author" />
</div>
<div class="form-group">
<label class="form-label">Content</label>
<InputTextArea class="form-input" rows="8" @bind-Value="_model.Content" />
</div>
<div class="form-group">
<label class="form-label">Title</label>
<InputText class="form-input" @bind-Value="_model.Title" />
</div>
<div class="form-group">
<label class="form-label">Author</label>
<InputText class="form-input" @bind-Value="_model.Author" />
</div>
<div class="form-group">
<label class="form-label">Content</label>
<InputTextArea class="form-input" rows="8" @bind-Value="_model.Content" />
</div>

<div class="flex gap-2">
<button type="submit" class="btn-primary">Create</button>
<a href="/blog" class="btn-secondary">Cancel</a>
</div>
</EditForm>
<div class="flex gap-2">
<button type="submit" class="btn-primary">Create</button>
<a href="/blog" class="btn-secondary">Cancel</a>
</div>
</EditForm>
</div>

@code {
private readonly PostFormModel _model = new();
private string? _error;
private readonly PostFormModel _model = new();
private string? _error;

private async Task HandleSubmit()
{
var result = await Sender.Send(new CreateBlogPostCommand(_model.Title, _model.Content, _model.Author));
if (result.Success)
Navigation.NavigateTo("/blog");
else
_error = result.Error;
}
private async Task HandleSubmit()
{
var result = await Sender.Send(new CreateBlogPostCommand(_model.Title, _model.Content, _model.Author));
if (result.Success)
Navigation.NavigateTo("/blog");
else
_error = result.Error;
}

private sealed class PostFormModel
{
[System.ComponentModel.DataAnnotations.Required]
public string Title { get; set; } = string.Empty;
private sealed class PostFormModel
{
[System.ComponentModel.DataAnnotations.Required]
public string Title { get; set; } = string.Empty;

[System.ComponentModel.DataAnnotations.Required]
public string Author { get; set; } = string.Empty;
[System.ComponentModel.DataAnnotations.Required]
public string Author { get; set; } = string.Empty;

[System.ComponentModel.DataAnnotations.Required]
public string Content { get; set; } = string.Empty;
}
[System.ComponentModel.DataAnnotations.Required]
public string Content { get; set; } = string.Empty;
}
}
4 changes: 2 additions & 2 deletions src/Web/Features/BlogPosts/Delete/ConfirmDeleteDialog.razor
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
<p class="text-red-600 dark:text-red-400 text-sm">This action cannot be undone.</p>
</div>
<div class="flex gap-2 justify-end">
<button class="px-4 py-2 rounded font-medium bg-red-600 text-white hover:bg-red-700 transition" @onclick="OnConfirm">Delete</button>
<button class="px-4 py-2 rounded font-medium border border-gray-300 dark:border-gray-600 text-gray-900 dark:text-gray-50 hover:bg-gray-100 dark:hover:bg-gray-700 transition" @onclick="OnCancel">Cancel</button>
<button class="btn-destructive" @onclick="OnConfirm">Delete</button>
<button class="btn-secondary" @onclick="OnCancel">Cancel</button>
</div>
</div>
</div>
Expand Down
7 changes: 2 additions & 5 deletions src/Web/Features/BlogPosts/Edit/Edit.razor
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
@page "/blog/edit/{Id:guid}"
@using MyBlog.Web.Features.BlogPosts.Edit
@inject ISender Sender
@inject NavigationManager Navigation
@rendermode InteractiveServer
@attribute [Authorize(Roles = "Author,Admin")]

<PageTitle>Edit Post</PageTitle>

<h1 class="text-3xl font-bold mb-6">Edit Post</h1>
<PageHeadingComponent HeaderText="Edit Post" Level="1" TextColorClass="text-primary-900 dark:text-primary-50" />

@if (_error is not null)
{
Expand All @@ -21,7 +18,7 @@
{
<div class="alert-warning" role="alert">
<div>
<strong class="font-semibold">Concurrency Conflict:</strong> This post was modified by another user.
<strong class="font-semibold">Concurrency Conflict:</strong> This post was modified by another user.
Please reload the page to get the latest version.
</div>
<button type="button" class="alert-dismiss" @onclick="() => _concurrencyError = false">&times;</button>
Expand Down
Loading
Loading