Skip to content

Commit 265c9d7

Browse files
author
DavidQ
committed
Apply final Asset Manager V2 code review cleanup and test separation - PR_26126_108-asset-manager-v2-final-code-review-cleanup
1 parent 090c6dc commit 265c9d7

7 files changed

Lines changed: 156 additions & 14749 deletions

‎docs/dev/reports/PR_26126_108_asset_manager_v2_final_code_review_notes.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# PR_26126_108 Final Code Review Notes
2+
3+
## Scope
4+
- Applied final cleanup using `docs/dev/PROJECT_INSTRUCTIONS.md` as the repository source of truth.
5+
- Kept changes limited to Asset Manager V2 runtime cleanup and required PR_26126_108 reports/shared review artifacts.
6+
- Did not change `package.json`.
7+
- Did not change `tests/playwright/PreviewGeneratorV2Baseline.spec.mjs`.
8+
- Did not modify sample JSON.
9+
10+
## Dead Code Cleanup
11+
- Removed the unused `currentToolState()` method from `AssetManagerV2App`.
12+
- Removed the now-unused `ASSET_MANAGER_TOOL_ID` constant.
13+
- Verified no remaining `exportAssets`, `exportToolState`, `copyAssetsJson`, `currentToolState`, or `ASSET_MANAGER_TOOL_ID` references remain in Asset Manager V2 source/tests.
14+
15+
## Test Ownership
16+
- `tests/playwright/AssetManagerV2.spec.mjs` remains the SSoT for Asset Manager V2 Playwright coverage.
17+
- `tests/playwright/PreviewGeneratorV2Baseline.spec.mjs` contains no Asset Manager V2-specific behavior/tests.
18+
- Keyboard-navigation cleanup remains intact in active Asset Manager V2 source/tests.
19+
20+
## Validation
21+
- Playwright impacted: Yes.
22+
- `npm run test:workspace-v2` passed 14/14 tests.
23+
- Full samples smoke test was skipped because this PR is scoped to Asset Manager V2 cleanup and does not modify shared sample loaders/frameworks or sample JSON.

‎docs/dev/reports/PR_26126_108_asset_manager_v2_manual_validation_notes.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# PR_26126_108 Manual Validation Notes
2+
3+
## Commands
4+
- `node --check tools/asset-manager-v2/js/AssetManagerV2App.js`
5+
- `rg -n "ASSET_MANAGER_TOOL_ID|currentToolState\\(|exportAssets\\(|exportToolState\\(|copyAssetsJson\\(" tools\asset-manager-v2 tests\playwright\AssetManagerV2.spec.mjs tests\playwright\PreviewGeneratorV2Baseline.spec.mjs`
6+
- `rg -n "KeyA|KeyS|KeyD|KeyW|ArrowLeft|ArrowRight|ArrowUp|ArrowDown|Home|End|PageUp|PageDown|Enter|keydown|keyup|data-asset-tile-id" tools\asset-manager-v2 tests\playwright\AssetManagerV2.spec.mjs`
7+
- `rg -n "Asset Manager V2|asset-manager-v2|openAssetManagerV2|openWorkspaceV2|workspace=UAT|workspace=uat|AssetManagerV2" tests\playwright\PreviewGeneratorV2Baseline.spec.mjs`
8+
- `git diff -- package.json tests\playwright\PreviewGeneratorV2Baseline.spec.mjs`
9+
- `git diff --check`
10+
- `npm run test:workspace-v2`
11+
12+
## Results
13+
- `npm run test:workspace-v2` passed 14/14 Playwright tests.
14+
- Dead Asset Manager V2 export/copy/toolState helper methods are absent from active source/tests.
15+
- Active Asset Manager V2 source/tests contain no listed keyboard-navigation key handling or assertions.
16+
- `tests/playwright/AssetManagerV2.spec.mjs` remains the dedicated Asset Manager V2 Playwright suite.
17+
- `tests/playwright/PreviewGeneratorV2Baseline.spec.mjs` has no Asset Manager V2-specific behavior/tests and has no PR_26126_108 diff.
18+
- `package.json` has no PR_26126_108 diff.
19+
- Full samples smoke test skipped: sample JSON and shared sample loading are out of scope for this Asset Manager V2 cleanup PR.
20+
- No sample JSON was modified.

‎docs/dev/reports/codex_changed_files.txt‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,16 @@
11
# git status --short
2-
M docs/dev/reports/PR_26126_093_asset_manager_v2_manual_validation_notes.md
3-
M docs/dev/reports/PR_26126_093_asset_manager_v2_palette_sample_notes.md
4-
M docs/dev/reports/PR_26126_096_asset_manager_v2_manual_validation_notes.md
5-
M docs/dev/reports/PR_26126_096_asset_manager_v2_temporary_uat_session_root_notes.md
6-
M docs/dev/reports/PR_26126_097_asset_manager_v2_font_preview_notes.md
7-
M docs/dev/reports/PR_26126_105_asset_manager_v2_launch_guard_notes.md
8-
M docs/dev/reports/PR_26126_105_asset_manager_v2_manual_validation_notes.md
9-
AM docs/dev/reports/PR_26126_107_asset_manager_v2_code_review_cleanup_notes.md
10-
AM docs/dev/reports/PR_26126_107_asset_manager_v2_manual_validation_notes.md
11-
M docs/dev/reports/codex_changed_files.txt
12-
M docs/dev/reports/codex_review.diff
13-
MM docs/dev/reports/coverage_changed_js_guardrail.txt
14-
MM docs/dev/reports/playwright_v8_coverage_report.txt
15-
M tests/playwright/AssetManagerV2.spec.mjs
16-
M tools/asset-manager-v2/js/AssetManagerV2App.js
17-
M tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js
2+
M docs/dev/reports/coverage_changed_js_guardrail.txt
3+
M docs/dev/reports/playwright_v8_coverage_report.txt
4+
M tools/asset-manager-v2/js/AssetManagerV2App.js
5+
?? docs/dev/reports/PR_26126_108_asset_manager_v2_final_code_review_notes.md
6+
?? docs/dev/reports/PR_26126_108_asset_manager_v2_manual_validation_notes.md
187

198
# git ls-files --others --exclude-standard
20-
(no output)
9+
docs/dev/reports/PR_26126_108_asset_manager_v2_final_code_review_notes.md
10+
docs/dev/reports/PR_26126_108_asset_manager_v2_manual_validation_notes.md
2111

2212
# git diff --stat
23-
.../PR_26126_107_asset_manager_v2_code_review_cleanup_notes.md | 1 +
24-
.../reports/PR_26126_107_asset_manager_v2_manual_validation_notes.md | 1 +
25-
docs/dev/reports/coverage_changed_js_guardrail.txt | 2 +-
26-
docs/dev/reports/playwright_v8_coverage_report.txt | 4 ++--
27-
tests/playwright/AssetManagerV2.spec.mjs | 2 +-
28-
tools/asset-manager-v2/js/services/TemporaryUatWorkspace.js | 3 ++-
29-
6 files changed, 8 insertions(+), 5 deletions(-)
13+
docs/dev/reports/coverage_changed_js_guardrail.txt | 2 +-
14+
docs/dev/reports/playwright_v8_coverage_report.txt | 8 +++-----
15+
tools/asset-manager-v2/js/AssetManagerV2App.js | 11 -----------
16+
3 files changed, 4 insertions(+), 17 deletions(-)

0 commit comments

Comments
 (0)