Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(FR-244): session renaming in the session detail panel #3034

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

yomybaby
Copy link
Member

@yomybaby yomybaby commented Jan 15, 2025

resolves #3035 (FR-244)

Enhances session name editing functionality with improved validation and user experience:

  • Adds form-based editing interface with validation rules for session names
  • Implements escape key to cancel editing
  • Shows visual feedback during editing with a down-left corner icon
  • Maintains consistent validation rules between initial creation and editing
  • Enables editing session names directly in the session detail drawer
  • Restricts editing to session owners and non-preparing states
  • Prevents accidental drawer closure with keyboard shortcuts while editing

Checklist:

  • Test case: Verify session name validation rules
  • Test case: Check escape key cancels editing
  • Test case: Confirm editing works in session detail drawer
  • Test case: Verify only session owners can edit names
  • Test case: Confirm editing is disabled during session preparation

@github-actions github-actions bot added the size:L 100~500 LoC label Jan 15, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

github-actions bot commented Jan 15, 2025

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
4.94% (-0.01% 🔻)
398/8054
🔴 Branches
4.27% (-0.01% 🔻)
239/5591
🔴 Functions
2.96% (-0% 🔻)
78/2633
🔴 Lines
4.86% (-0.01% 🔻)
383/7880

Test suite run success

124 tests passing in 14 suites.

Report generated by 🧪jest coverage report action from 7d99ac1

@yomybaby yomybaby changed the title feat: session renaming in the session detail panel feat(FR-244): session renaming in the session detail panel Jan 15, 2025
@yomybaby yomybaby force-pushed the feature/session-renaming-in-detail-panel branch from 6fa6038 to da5f2aa Compare January 15, 2025 07:27
@github-actions github-actions bot added the area:lib Library and SDK related issue. label Jan 15, 2025
@yomybaby yomybaby marked this pull request as ready for review January 15, 2025 07:28
@ironAiken2 ironAiken2 requested review from ironAiken2, inureyes and agatha197 and removed request for inureyes and agatha197 January 15, 2025 07:35
@yomybaby yomybaby requested a review from agatha197 January 15, 2025 07:36
@yomybaby yomybaby force-pushed the feature/session-renaming-in-detail-panel branch from da5f2aa to f4e4a4f Compare January 15, 2025 08:08
@yomybaby yomybaby requested a review from ironAiken2 January 15, 2025 08:09
Copy link
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

It can be modified as a duplicate of another currently existing session name. teams

Copy link
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member Author

Thank you for mentioning it. Currently, modify_compute_session has a bug which are not validate name duplication. The feature to prevent duplicate names in the WebUI will be addressed after fixing the related API bug.

Copy link

graphite-app bot commented Jan 15, 2025

Merge activity

resolves #3035 (FR-244)

Enhances session name editing functionality with improved validation and user experience:
- Adds form-based editing interface with validation rules for session names
- Implements escape key to cancel editing
- Shows visual feedback during editing with a down-left corner icon
- Maintains consistent validation rules between initial creation and editing
- Enables editing session names directly in the session detail drawer
- Restricts editing to session owners and non-preparing states
- Prevents accidental drawer closure with keyboard shortcuts while editing

**Checklist:**
- [ ] Test case: Verify session name validation rules
- [ ] Test case: Check escape key cancels editing
- [ ] Test case: Confirm editing works in session detail drawer
- [ ] Test case: Verify only session owners can edit names
- [ ] Test case: Confirm editing is disabled during session preparation
@yomybaby yomybaby force-pushed the feature/session-renaming-in-detail-panel branch from f4e4a4f to 7d99ac1 Compare January 15, 2025 08:52
@graphite-app graphite-app bot merged commit 7d99ac1 into main Jan 15, 2025
6 checks passed
@graphite-app graphite-app bot deleted the feature/session-renaming-in-detail-panel branch January 15, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:lib Library and SDK related issue. size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session renaming in the Session detail panel
2 participants