Skip to content

Conversation

qjhuangAWS
Copy link

@qjhuangAWS qjhuangAWS commented Jul 16, 2025

Problem

Currently, there is only run and reject available when users are asking q to run a command. There is no option to change the command.
Screenshot 2025-07-16 at 3 52 58 PM

Solution

Added modify button with changes mainly to chat-item-card.ts and chat-item-card-content.ts that turns the content into an editable space when modify is clicked.
Screenshot 2025-07-16 at 3 56 50 PM
Screenshot 2025-07-16 at 3 57 06 PM
Screenshot 2025-07-16 at 3 57 15 PM

Core Functionality

Editable Chat Item Content System

  • What: Added editable property support to ChatItem and ChatItemCardContent components with textarea-based editor for shell commands with auto-resize. Updates the editable state and rebuilds UI components accordingly.
  • Why: Enables dynamic switching between read-only display and editable input modes for chat content

UI Changes

  • What: Different button configurations for view vs edit modes, and maintains shell command content when switching between modes
  • Why: Provides contextually appropriate actions (Run/Reject/Modify vs Accept/Cancel), and prevents data loss during editing operations
  • Example:
  • View mode: [Run] [Reject] [Modify]
  • Edit mode: [Accept] [Cancel]

Tests

  • I have tested this change on VSCode
  • I have tested this change on JetBrains
  • I have added/updated the documentation (if applicable)

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

- Fix nullable boolean expressions with explicit comparisons
- Fix string concatenation using template literals
- Fix non-null assertions with proper null checks
- Fix hasOwnProperty usage with Object.prototype method
- Remove unused imports
- Fix nullable boolean expressions with explicit equality checks
- Remove unused MynahEventNames import from test file
- Fix any value conditionals with explicit null checks
- Fix no-new rule violations with void operator
- All lint errors resolved for modify button functionality
- Automatically correct indentation spacing in test files
- Resolve final linting issues for push approval
@qjhuangAWS qjhuangAWS requested a review from a team as a code owner July 16, 2025 22:45
@qjhuangAWS qjhuangAWS changed the title Feature/modify button functionality New Feature: modify button - card components and function handling Jul 16, 2025
@qjhuangAWS qjhuangAWS changed the title New Feature: modify button - card components and function handling feat: modify button - card components and function handling Jul 16, 2025
Copy link
Contributor

@laileni-aws laileni-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix merge conflicts

…nd main

- Resolved conflicts in sample-data.ts by keeping both border and padding properties
- Updated main.ts with additional chat history entries
- All merge conflicts have been successfully resolved
@qjhuangAWS qjhuangAWS changed the title feat: modify button - card components and function handling feat: modify button - card components and function handling on mynah Jul 16, 2025
…merge

The merge conflict resolution in commit 033d717 accidentally removed the modify
button functionality from main.ts. This commit restores:

- onInBodyButtonClicked handler for modify-bash-command actions
- save-bash-command and cancel-bash-edit functionality
- reject-bash-command and run-bash-command handlers
- Shell command example in HEADER_TYPES case
- SHELL_WITH_MODIFY command case

All functionality from the original feature/modify-button-functionality
branch has been restored.
- Updated visual snapshots to reflect UI changes from new modify button feature
- Fixed failing e2e tests that were caused by layout changes
- All tests now pass (126 passed, 4 skipped)
- Remove unused mockTextarea variable
- Replace || with ?? for stricter boolean check
- Fixes pre-push hook failures
@qjhuangAWS qjhuangAWS closed this Jul 17, 2025
@qjhuangAWS qjhuangAWS reopened this Jul 17, 2025
- Updated package.json to use exact version 1.50.0 for both @playwright/test and playwright
- This matches the Docker environment version used in CI/CD
- Regenerated all snapshots with the correct Playwright version
- All 126 tests now pass with updated snapshots
@avi-alpert
Copy link
Contributor

I'm not a fan of thousands of lines of unrelated formatting changes being included in this PR. When future developers make changes to these files the same issue will reoccur. If you want to open a separate PR that fixes the linting/formatting for the example folder I think that would be great

Copy link
Contributor

@laileni-aws laileni-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update PR description and documentation according to your changes and remove the commented code

@avi-alpert
Copy link
Contributor

This PR completely breaks the Amazon Q chat experience. Was this tested in VS Code as claimed in the PR description?

Screen.Recording.2025-07-21.at.4.28.43.PM.mov

@qjhuangAWS
Copy link
Author

qjhuangAWS commented Jul 21, 2025

Most of comments are resolved, currently working on failing render content test case and looking at regression failing in vsc before next push

@qjhuangAWS
Copy link
Author

This PR completely breaks the Amazon Q chat experience. Was this tested in VS Code as claimed in the PR description?

Screen.Recording.2025-07-21.at.4.28.43.PM.mov

was able to fix this problem yesterday, still looking at the snapshot error and will be going over the code with mentor Ashish toda

@ashishrp-aws ashishrp-aws marked this pull request as draft July 22, 2025 21:33
@qjhuangAWS qjhuangAWS marked this pull request as ready for review August 5, 2025 18:57
@qjhuangAWS qjhuangAWS changed the base branch from main to feature/modify-chat-item-card August 6, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants