feat: add inline editing, deletion, implemented status, and code refactoring#13
Merged
feat: add inline editing, deletion, implemented status, and code refactoring#13
Conversation
This commit implements user-requested features to delete and edit recommendation cards directly in the interface. Features added: 1. Delete Recommendation Cards - Add delete button (✕ Delete) to each card - Confirmation dialog before deletion - AJAX removal from DOM and storage - Restore empty state when last card deleted - Success/error messaging 2. Inline Editing with Autosave - All text fields are contenteditable (title, description, ideas) - Autosave after 1 second of no typing - Also saves on blur (clicking away) - Visual feedback: "Saving..." → "Saved" - Hover outline to indicate editability - Focus outline for active editing - Prevent Enter key in title fields 3. UX Improvements - Red delete link with hover effect - Cursor changes to text on hover over editable fields - Subtle save indicator - Error handling with user feedback - Maintains data-title attribute after title edits Files modified: - ai_content_strategy.routing.yml: Added delete_card and save_card routes - templates/ai-content-strategy-recommendations-items.html.twig: Added contenteditable, data attributes, delete button, save indicator - src/Controller/ContentStrategyController.php: Added deleteCard() and saveCard() methods - js/content-ideas.js: Added delete and edit behaviors with AJAX - css/content-strategy.css: Added editable field styling Technical details: - Uses native contenteditable (no WYSIWYG editor) - Debounced autosave (1000ms) - strip_tags() for security - Updates key-value store on changes - Proper HTTP error codes via getHttpStatusFromException() - Reattaches Drupal behaviors after DOM changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add new AJAX endpoint to delete individual content ideas from recommendation cards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implement controller method to delete individual content ideas from recommendation cards. Removes idea from array, re-indexes, and returns AJAX response to remove row from DOM. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…xport, and multi-instance support
- Add delete button to each content idea row (hidden until hover)
- Move card delete button from bottom to header inline with title
- Add CSV export button with proper column ordering
- Fix multi-instance support by removing hardcoded IDs
- Use inline SVG icons for portability across themes
Individual idea deletion:
- New delete button in each content idea table row
- Hidden by default (opacity: 0), appears on row hover
- Contextual confirmation shows idea text being deleted
- JavaScript behavior processes AJAX response
Card delete repositioning:
- Moved from recommendation-actions to recommendation-header
- Positioned inline with card title using flexbox
- Subtle 60% opacity, full on hover
- Better visual hierarchy
CSV export:
- Frontend JavaScript implementation
- Column order: Content Idea, Category, Priority, Recommendation Title
- Removed Description column to reduce repetition
- Proper CSV escaping for commas, quotes, newlines
- Downloads as content-strategy-YYYY-MM-DD.csv
Multi-instance support:
- Removed getElementById('generation-progress')
- Changed message IDs to timestamp-based
- All selectors use classes/data attributes
- Supports multiple form instances on same page
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
jjroelofs
force-pushed
the
jur/1.x/add-delete-and-edit-cards
branch
from
0543b74 to
88c5bc1
Compare
added 4 commits
January 21, 2026 09:56
- Change button text from "Add" to "Generate" for consistency - Add inline save indicator with Drupal core throbber and checkmark - Add green flash animation on successful save - Add implemented checkbox for content ideas with strikethrough styling - Add link field for implemented ideas with add/edit functionality - Update CSV export to include Implemented and Link columns - Extract SVG icons into Twig variables for reusability - Add component templates for link display and input - Pass rendered templates and translations to JS via drupalSettings - Use Drupal's native button classes for consistent admin theme styling
JavaScript: - Split content-ideas.js (1012 lines) into 7 modular files: - content-strategy-utils.js: Shared utilities and AJAX handler - content-strategy-generate.js: Generate button behaviors - content-strategy-editable.js: Editable field auto-save - content-strategy-delete.js: Card and idea deletion - content-strategy-checkbox.js: Implemented checkbox toggle - content-strategy-links.js: Link add/edit functionality - content-strategy-export.js: CSV export - Each module uses Drupal behaviors pattern for independent attachment CSS: - Split content-strategy.css into 7 component files: - base.css: Container, status area, actions, empty state - cards.css: Card layout, header, priority badges - table.css: Content ideas table, implemented row styling - checkbox.css: Custom checkbox with checkmark animation - links.css: Link display, edit button, input form - delete.css: Delete buttons for cards and ideas - editable.css: Editable fields with save indicators - Added icons.css with CSS mask-image for colorable SVG icons Icons: - Replaced inline SVG with CSS mask-image approach - Icons now inherit color from parent via currentColor - Updated templates to use cs-icon classes - Simplified module preprocess (no longer passes icons via JS) Constants: - Added src/Constants.php with centralized magic strings - Includes CSS selectors, field names, data attributes, icon names
- Fix save indicator not being found when placed as sibling element (check both inside field and next sibling for indicator) - Add button--small class to "Add link" button in template and JS
jjroelofs
changed the title
added 9 commits
January 21, 2026 10:19
- Replace \Drupal::request() with injected RequestStack service - Remove unused $deleted_idea variable in deleteIdea method - Fixes PHPCS and drupal-check linting warnings
When new cards or ideas are generated via AJAX, the editable fields, checkboxes, and delete buttons were not functional because their Drupal behaviors were not attached to the new DOM elements. Added Drupal.attachBehaviors() call in the AJAX success handler to properly initialize all interactive elements on new content.
The generateMore method was creating simple table rows without the interactive elements (editable fields, checkboxes, delete buttons, link areas). This caused newly generated ideas to be non-functional. Now generates complete HTML structure matching the Twig template, including: - Contenteditable div with proper data attributes - Hidden link area with add link button - Implemented checkbox with visual styling - Delete button with trash icon Also pre-computes translated strings to satisfy Drupal coding standards.
Comprehensive plan covering: - Phase 1: Extract services (RecommendationStorage, IdeaRowBuilder, AjaxResponseBuilder) - Phase 2: Move inline HTML to Twig templates - Phase 3: Replace raw fetch() with Drupal.ajax() - Phase 4: Cleanup and validation Each phase includes sanity checks and quality controls.
- Extract 3 services from controller: - RecommendationStorageService: handles key-value store CRUD - IdeaRowBuilder: renders idea rows using Twig templates - AjaxResponseBuilder: standardizes AJAX response creation - Replace all fetch() with Drupal.ajax() in JS files: - content-strategy-delete.js - content-strategy-links.js - content-strategy-checkbox.js - content-strategy-editable.js - content-strategy-generate.js - content-strategy-utils.js (simplified) - Move inline HTML to Twig templates: - Create templates/components/idea-row.html.twig - Use @ai_content_strategy namespace for includes - Controller returns HtmlCommand with template-rendered HTML - Update CLAUDE.md as architecture documentation
Replace title-based card identification and index-based idea identification
with UUIDs for more reliable CRUD operations. This prevents issues with
special characters in titles and ensures stable references during editing.
Changes:
- Add UUID generation for cards and ideas in RecommendationStorageService
- Update routes to use {uuid} and {idea_uuid} parameters
- Update all JS handlers to use data-uuid and data-idea-uuid attributes
- Add migration logic to assign UUIDs to existing stored data
- Update Twig templates to pass UUID data attributes
- Replace @deprecated with @internal annotation on findCardIndex() method since it's still used internally for backwards compatibility - Fix undefined $title variable in generateMore() by fetching card data before building the AI prompt
- Remove unused $card_title variable in generateMore() since title is already fetched earlier in the method - Remove unused $section loop variable in ensureUuids() and ensureIdeaUuids()
nechai
reviewed
Jan 21, 2026
| * Centralizes magic strings to improve maintainability and reduce | ||
| * duplication across the codebase. | ||
| */ | ||
| final class Constants { |
Collaborator
There was a problem hiding this comment.
I don't see any usage of this class...
nechai
reviewed
Jan 21, 2026
| * Key-value storage keys. | ||
| */ | ||
| const KV_COLLECTION = 'ai_content_strategy.recommendations'; | ||
| const KV_KEY = 'recommendations'; |
Collaborator
There was a problem hiding this comment.
KV_KEY is defined in ContentStrategyController and in RecommendationStorageService.
I see other constants that are the same in different classes (like KV_COLLECTION, etc.)
nechai
reviewed
Jan 21, 2026
| // Build the HTML for the new rows. | ||
| $html = $this->renderer->renderRoot($rows); | ||
| // Update the stored data with timestamp. | ||
| $timestamp = (int) $this->time->getCurrentTime(); |
Collaborator
There was a problem hiding this comment.
(int) type hint is redundant here as time->getCurrentTime() always return the integer (I see the same in a couple of other places)
nechai
reviewed
Jan 21, 2026
| */ | ||
| class IdeaRowBuilder { | ||
|
|
||
| use StringTranslationTrait; |
Collaborator
There was a problem hiding this comment.
I don't see the usage of this trait...
nechai
reviewed
Jan 21, 2026
| * A render array for the link area. | ||
| */ | ||
| public function buildLinkArea(string $section, string $card_uuid, string $idea_uuid, string $link, bool $implemented = TRUE): array { | ||
| if ($link) { |
Collaborator
There was a problem hiding this comment.
could be replaced with an inline condition (return $link ? [...] : [...])
Collaborator
|
Hi @jjroelofs, after manual testing, I found the next steps failed: Card Deletion
Link Field
|
- Fix Edit link not working after AJAX save (override success method) - Fix CSV export button not appearing after first generation - Fix delete card button always visible (now shows only on hover) - Remove unused Constants.php class - Remove unused StringTranslationTrait from IdeaRowBuilder - Remove redundant (int) casts from Time service calls
Contributor
Author
|
Thanks! All feedback was addressed |
jjroelofs
pushed a commit
that referenced
this pull request
Apr 7, 2026
Generation commands — the reviewer's hard requirement (#3/#4): - Add --dry-run to acs:generate, acs:generate:more, acs:generate:add - acs:generate --dry-run generates recommendations but does not save, warns if existing data would be overwritten - acs:generate:more --dry-run shows generated ideas without appending - acs:generate:add --dry-run shows new cards without saving - All three return dry_run: true in YAML envelope Field clearing (#12): - CardCommands: replace !empty() with !== NULL && !== '' checks, allowing --title="" and --description="" to clear fields - CategoryCommands: allow --instructions="" and --description="" to clear to empty string (label still requires non-empty) Report envelope (NEW #1): - acs:report now uses $this->success() for proper message field instead of raw $this->yaml() CSV robustness (#13): - csvRow() strips embedded newlines (RFC 4180 compliance) Export documentation (#15): - CLI\Help documents that raw output is returned without envelope when --file is not used (intentional for piping) E2E test helpers (#9): - assert_success() now checks YAML 'success: true' (not exit code) - assert_fail() now checks YAML 'success: false' (not exit code) - Both use || true to capture output regardless of exit code E2E runner robustness (NEW #5, #6): - trap 'rm -rf "$SITE_DIR"' EXIT prevents temp dir leak on failure - Runner continues through all test files even if one fails - Reports aggregate results and lists failed files at the end E2E test quality (NEW #3, #4, #22): - test-settings.sh verifies persisted value after set - test-generation.sh checks for specific 'success: false' on health - All expected-failure Drush calls have || true appended - test-settings.sh tests assert_dry_run on dry-run response CI (#19): - Add 'Generate test summary' step with if: always() that writes pass/fail results to GITHUB_STEP_SUMMARY
jjroelofs
added a commit
that referenced
this pull request
Apr 7, 2026
…parity (#19) * feat: add comprehensive Drush CLI commands for GUI/TUI feature parity Implements 21 Drush commands across 8 files, closing all GUI feature gaps for AI-Agent readiness. Commands follow drush_webmaster patterns: YAML output, consistent response envelope, --dry-run support, admin user switching, and PHP 8 attributes. Commands: acs:report, acs:report:card, acs:report:status, acs:sitemap, acs:card:edit, acs:card:delete, acs:idea:edit, acs:idea:implement, acs:idea:delete, acs:category:list/get/create/update/delete, acs:settings:get/set, acs:export, acs:generate, acs:generate:more, acs:generate:add, acs:health. Closes #18 * fix: remove trailing newline from CSV export Prevents an empty final row when CSV output is parsed by standard CSV parsers. * feat: AI skill files, E2E tests, setup command, and architecture alignment Implements #18 — full GUI/TUI parity with AI-agent integration following patterns from rl#32, dxpr_builder#4468, and dxpr_theme#803 + dxpr_theme_helper#49. Architecture fixes: - Replace global pre-command hook with explicit switchToAdmin() per command - Remove create() factory methods from all 7 command classes - Add --dry-run to acs:card:edit, acs:idea:edit, acs:idea:implement - Add switchToAdmin() calls to all 22 command methods New: AI skill files & setup command (Phase 5): - SetupCommands.php with acs:setup-ai (--host, --check) - .claude/skills/acs/SKILL.md with preamble auto-discovery - .agents/skills/acs/SKILL.md condensed cross-agent version - hook_requirements() for skill file staleness detection New: E2E test suite (Phase 6): - scripts/e2e/ with _helpers.sh, run-e2e-tests.sh, 8 test files - docker-compose.yml e2e-test service, drush-e2e CI job Documentation (Phase 7): - desc.html: AI prompting examples for end users - README.md: Drush CLI section, command table, AI integration Lint fixes: - Remove PHPCompatibility (incompatible with phpcs v4) - Fix phpcbf exit code handling, array indentation * fix: address all PR review feedback (critical, high, and medium) Critical fixes: - Fix operator precedence bug in ReportCommands::reportStatus() — timestamp was returned as raw int instead of ISO 8601 string (#1) - Remove static \Drupal:: calls in GenerationCommands, inject UuidInterface and use $this->storage instead of static service calls (#2) - Remove unused CategorySchemaBuilder and ConfigFactoryInterface deps Blocking fixes: - acs:generate --category now regenerates (replaces) via new regenerateCategory() method + RecommendationStorageService::replaceSection(), distinct from acs:generate:add which appends (#2 blocking) - hook_requirements() now only flags installed-but-outdated files, not missing files from partial --host installs. Uses REQUIREMENT_INFO for completely uninstalled state (#3 blocking) - Add dry_run: true to all dry-run YAML envelopes: card:delete, idea:delete, category:create/update/delete, settings:set (#4 blocking) High fixes: - Validate --file path in ExportCommands before writing (#5) - Log warning on switchToAdmin() failure instead of silent catch (#6) - Replace recursive regex with simpler strpos+json_decode for JSON extraction from AI responses (#7) - Rewrite all 8 E2E test files with proper fixtures (pre-populated recommendations created in run-e2e-tests.sh). Tests now cover happy-path CRUD: card edit/delete, idea edit/implement/undo/delete, export in yaml/json/csv, report filtering by category/priority (#8) - Fix assert_has to use grep -F for literal matching (#9/#26) - Remove vacuous assertion in test-generation.sh (#10) Medium fixes: - Add message field to successList() envelope (#11) - Remove unused Yaml import from ExportCommands, use $this->yaml() (#17) - Pin E2E runner to 11.1.x-dev instead of unstable 11.x-dev (#18) - Fix Claude SKILL.md frontmatter: singular trigger field, remove name/version fields not in spec (#20) - Fix openai.yaml: use name field instead of display_name (#21) - Fix formatCard type safety: use empty string fallback (#23) - Check copy() return value in SetupCommands::installFile() (#29) * fix: resolve PHPStan errors and E2E test ordering issue PHPStan (drupal-check) — 7 errors → 0: - Add @var annotations for loadMultiple()/loadByProperties() returns so PHPStan knows the concrete RecommendationCategory type - Add @var annotation for $storage->create() return - Use static closures with explicit int return type on uasort callbacks E2E (drush-e2e) — 2 failures → 0: - test-export.sh assertions now check for stable values (category label "Content Gaps", "priority:") instead of the fixture card title which gets mutated by test-cards.sh running earlier in the suite * fix: use literal string match in test-generation.sh (grep -F) * fix: test-report.sh assertions use immutable values (uuid, priority) Report tests run after cards/ideas tests which mutate fixture titles and idea text. Switch assertions from mutable values (card title, idea text) to immutable ones (card UUID, priority field) that survive cross-test mutations. * fix: address all remaining review feedback comprehensively Generation commands — the reviewer's hard requirement (#3/#4): - Add --dry-run to acs:generate, acs:generate:more, acs:generate:add - acs:generate --dry-run generates recommendations but does not save, warns if existing data would be overwritten - acs:generate:more --dry-run shows generated ideas without appending - acs:generate:add --dry-run shows new cards without saving - All three return dry_run: true in YAML envelope Field clearing (#12): - CardCommands: replace !empty() with !== NULL && !== '' checks, allowing --title="" and --description="" to clear fields - CategoryCommands: allow --instructions="" and --description="" to clear to empty string (label still requires non-empty) Report envelope (NEW #1): - acs:report now uses $this->success() for proper message field instead of raw $this->yaml() CSV robustness (#13): - csvRow() strips embedded newlines (RFC 4180 compliance) Export documentation (#15): - CLI\Help documents that raw output is returned without envelope when --file is not used (intentional for piping) E2E test helpers (#9): - assert_success() now checks YAML 'success: true' (not exit code) - assert_fail() now checks YAML 'success: false' (not exit code) - Both use || true to capture output regardless of exit code E2E runner robustness (NEW #5, #6): - trap 'rm -rf "$SITE_DIR"' EXIT prevents temp dir leak on failure - Runner continues through all test files even if one fails - Reports aggregate results and lists failed files at the end E2E test quality (NEW #3, #4, #22): - test-settings.sh verifies persisted value after set - test-generation.sh checks for specific 'success: false' on health - All expected-failure Drush calls have || true appended - test-settings.sh tests assert_dry_run on dry-run response CI (#19): - Add 'Generate test summary' step with if: always() that writes pass/fail results to GITHUB_STEP_SUMMARY * fix: data-loss regression, dry-run API waste, sitemap E2E, CI pipefail #12 BLOCKER — CategoryCommands data-loss regression: - Change option defaults from '' to NULL for label, instructions, description. Omitted options are now distinguishable from explicitly empty. A label-only update no longer silently clears instructions. - CardCommands: same fix — title/description defaults changed to NULL. - Add empty-label validation: --label="" returns an error. #3 — --dry-run no longer calls the AI API: - acs:generate --dry-run returns enabled categories and warns about overwrite — zero AI calls. - acs:generate:more --dry-run returns card info and existing idea count — zero AI calls. - acs:generate:add --dry-run returns category info — zero AI calls. - acs:generate --category --dry-run returns existing card count — zero AI calls. #4 / NEW #2 — regenerateCategory no longer wastes API quota: - New regenerateCategoryViaPrompt() makes a single category-specific AI call using categoryPromptBuilder, instead of calling generateRecommendations() for all categories and discarding the rest. acs:sitemap E2E — now tests against a real HTTP server: - run-e2e-tests.sh starts PHP built-in server (localhost:8888) before tests, adds it to cleanup trap. - test-report.sh runs acs:sitemap -l http://localhost:8888 and asserts success and content_types presence. CI pipefail: - Add set -o pipefail before tee in review.yml so a failing test run correctly fails the job even when piped. NEW #7 — acs:sitemap now uses $this->success() for proper envelope. NEW #8 — Remove dead TOTAL_PASS/TOTAL_FAIL/TOTAL_TESTS counters from run-e2e-tests.sh. * fix: sitemap E2E asserts HTTP connectivity, not sitemap existence acs:sitemap test now verifies the command reaches the PHP web server and handles the response (404 for missing sitemap.xml is expected without a sitemap module). Asserts 'sitemap.xml' in output to confirm actual HTTP fetch occurred (vs cURL 7 connection refused). * fix: system prompt regression, sitemap happy-path, dry-run E2E, code dedup System prompt regression (#1 from review 5): - regenerateCategoryViaPrompt() now reads global system_prompt from config and prepends it to the category system message, matching StrategyGenerator behavior. ConfigFactoryInterface re-injected. Code deduplication (observation #1 from review 4): - Extract callCategoryAi() and buildUserPrompt() shared helpers. Both regenerateCategoryViaPrompt() and generateForCategory() now delegate to callCategoryAi() instead of duplicating ~50 lines of prompt building, AI calling, and UUID processing. Sitemap happy-path (#2 from review 5): - run-e2e-tests.sh creates a static sitemap.xml with 5 URLs in the Drupal web root before starting the PHP server. - test-report.sh now asserts success: true, total_urls: 5, content_types presence, and URL content — a true happy-path test. - No more false-positive tests that pass on success: false output. --dry-run E2E coverage (#3 from review 5): - test-generation.sh now tests all four dry-run paths: acs:generate --dry-run, --category --dry-run, generate:add --dry-run, generate:more --dry-run. Each asserts success: true, dry_run: true, and path-specific fields (enabled_categories, existing_cards, etc.) Non-blocking details: - test-setup-ai.sh: add || true on --host=invalid expected-error call - review.yml: narrow CI summary grep to summary lines only * refactor: extract resolveIdea() and loadEnabledCategories() DRY helpers - IdeaCommands: extract resolveIdea() to eliminate 3 identical card+idea lookup guard blocks across editIdea, implementIdea, deleteIdea - RecommendationStorageService: add loadEnabledCategories() to centralize the load-enabled-sorted-by-weight pattern used in ReportCommands, ExportCommands, and GenerationCommands - ExportCommands: remove now-unused EntityTypeManagerInterface dependency --------- Co-authored-by: Jurriaan Roelofs <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds comprehensive inline editing, deletion capabilities, implemented status tracking, and significant code maintainability improvements to the AI Content Strategy recommendations UI.
Features
Code Refactoring
content-ideas.js(1012 lines) into 7 modular behavior filescontent-strategy.cssinto 8 component CSS filessrc/Constants.phpfor centralized magic stringsTest Plan
Setup
/admin/reports/ai/content-strategyInline Editing ✅
Card Deletion ❌
Idea Deletion ✅
Implemented Status ✅
Link Field ❌
CSV Export ✅
Visual Feedback